I forgot to Cc Johannes and Kalle... On Thu, Jan 19, 2017 at 09:15:09AM -0500, John W. Linville wrote: > On Thu, Jan 19, 2017 at 07:35:22AM -0500, David Arcari wrote: > > On 01/18/2017 11:45 AM, David Miller wrote: > > > From: David Arcari <darc...@redhat.com> > > > Date: Wed, 18 Jan 2017 08:34:05 -0500 > > > > > >> If the user executes 'ethtool -d' for an interface and the associated > > >> get_regs_len() function returns 0, the user will see a call trace from > > >> the vmalloc() call in ethtool_get_regs(). This patch modifies > > >> ethtool_get_regs() to avoid the call to vmalloc when the size is zero. > > >> > > >> Signed-off-by: David Arcari <darc...@redhat.com> > > > I think when the driver indicates this, it is equivalent to saying that > > > the operation isn't supported. > > > > > > Also, this guards us against ->get_regs() methods that don't handle > > > zero length requests properly. I see many which are going to do > > > really terrible things in that situation. > > > > > > Therefore, if get_regs_len() returns zero, treat it the safe as if the > > > ethtool operations were NULL. > > > > > > Thanks. > > > > That was actually the fix that I was originally considering, but it > > turns out > > there is a problem with it. > > > > I found that the vmalloc error was occurring because > > ieee80211_get_regs_len() in > > net/mac80211/ethtool.c was returning zero. The ieee80211_get_regs in > > the same > > file returns the hw version. It turns out that this information is used > > by the > > at76c50x-usb driver in the user space ethtool to report which HW variant > > is in > > use. Returning an error when regs_len() returns zero would break this > > functionality. > > > > -Dave > > I'm responsible for this mess. The original idea was for various > mac80211-based drivers to override the ethtool operation and provide > their own dump operation, but the mac80211 crowd never embraced > the idea. > > In the meantime, I added the default implementation which just > passed-up wdev->wiphy->hw_version as the version info for a 0-length > register dump. I then implemented a driver-specific regiser dump > handler for userland ethtool that would interpret the hardware version > information for the at76c50x-usb driver. > > So the net of it is, if we treat a return of 0 from get_regs_len() > as "not supported", we break this one driver-specific feature for > userland ethtool. Realistically, there are probably very few users > to care. But I can't guarantee that the number is zero. > > Possible solutions: > > -- break userland ethtool for at76c50x-usb > -- avoid 0-len allocation attempt (David Arcari's patch) > -- make allocator accept a 0 length value w/o oops'ing > -- change mac8011 code to return non-zero from get_regs_len() > > Thoughts? The last option holds a certain attraction, but I'm not > sure how to make it useful...? > > John > > -- > John W. Linville Someday the world will need a hero, and you > linvi...@tuxdriver.com might be all we have. Be ready. >
-- John W. Linville Someday the world will need a hero, and you linvi...@tuxdriver.com might be all we have. Be ready.
