search

Avnu / OpenAvnu

OpenAvnu - an Avnu sponsored repository for Time Sensitive Network (TSN and AVB) technology
468 stars 289 forks source link

igb_main: fix dma memory corruption #835

Closed jeremy-compostella closed 6 years ago

jeremy-compostella commented 6 years ago

This patch addresses two cases leading to DMA memory corruption and eventually kernel panics:

  1. If the ethernet interface has been up but is down when the AVB stack is started, the IGB_MAPBUF ioctl returns an address which is no more allocated leading to memory corruption which usually ends up in a Kernel panic. This patch controls that the DMA memory is allocated before it returns the address to the application.

  2. If the AVB application is up and running and the ethernet goes down (ifconfig eth0 down for example), the AVB application is not aware that the DMA memory it is using has been freed by the driver and it keeps using which leads to memory corruption and eventually kernel panics. This patch prevents the driver from freeing memory which is still in use by the application.

Signed-off-by: Jeremy Compostella jeremy.compostella@intel.com

pinealservo commented 6 years ago

Regarding #2, if the driver doesn't free the memory, does it still hold on to it to free later, or is it leaked? I guess a leak is probably preferable to corruption and kernel panic anyway, I'm just curious if there are still potential issues hiding here or if this takes care of them.

jeremy-compostella commented 6 years ago

Your right #2 isn't perfect but it is not that bad. The risk I see is a potential memory leak if the interface is turned down, then the AVB application is killed and then the driver is rmmod. As you said it is better to have a potential memory leak than a Kernel crash. As long as the driver is still running (even if the interface is down), the memory is still referenced and will be re-used again if the interface is turned up.

I am totally new to this driver, but from what I see it would require an architecture change to do things differently and avoid the current memory corruption issue in a proper way.

andrew-elder commented 6 years ago

@jeremy-compostella - I think the correct place to submit this pull request is actually here https://github.com/AVnu/igb_avb. I need to do a pull request from open-avb-next to master to remove some legacy directories that have be split off into their own git repos.

jeremy-compostella commented 6 years ago

Oh I see that it has been fixed already very recently (see https://github.com/AVnu/igb_avb/commit/ec4c4ed14d86ab011860208798ee6796e805de42).