Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.kernel > #67120

Bug#960702: [PATCH net] mlx4: Fix information leak on failure to read module EEPROM

Cross-posted to 2 groups.

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Mon, 2020-05-18 at 16:47 +0000, Saeed Mahameed wrote:
> On Sun, 2020-05-17 at 18:20 +0100, Ben Hutchings wrote:
> > mlx4_en_get_module_eeprom() returns 0 even if it fails.  This results
> > in copying an uninitialised (or partly initialised) buffer back to
> > user-space.
[...]
> I am not sure i see the issue in here, and why we need the partial
> memset ?
> first thing in this function we do:
> memset(data, 0, ee->len);
> 
> and then mlx4_get_module_info() will only copy valid data only on
> success.

Wow, sorry, I don't know how I missed that.  So this is not the bug I
was looking for.

> 
> > -		if (!ret) /* Done reading */
> > +		if (!ret) {
> > +			/* DOM was not readable after all */
> 
> actually if mlx4_get_module_info()  returns any non-negative value it
> means how much data was read, so if it returns 0, it means that this
> was the last iteration and we are done reading the eeprom.. 
> 
> so i would remove the above comment and the memset below is redundant
> since we already memset the whole buffer before the while loop.

Right.

> > +			memset(data + i, 0, ee->len - i);
> >  			return 0;
> > +		}
> >  
> >  		if (ret < 0) {
> >  			en_err(priv,
> >  			       "mlx4_get_module_info i(%d) offset(%d)
> > bytes_to_read(%d) - FAILED (0x%x)\n",
> >  			       i, offset, ee->len - i, ret);
> > -			return 0;
> > +			return ret;
> 
> I think returning error in here was the actual solution for your
> problem. you can verify by looking in the kernel log and verify you see
> the log message.

The original bug report (https://bugs.debian.org/960702) says that
ethtool reports different values depending on whether its output is
redirected.  Although returning all-zeroes for the unreadable part
might be wrong, it doesn't explain that behaviour.

Perhaps if the timing of the I²C reads is marginal, varying numbers of
bytes of DOM information might be readable?  But I don't see how
redirection of ethtool's output would affect that.  It uses a single
ioctl to read everything, and the kernel controls timing within that.

So I am mystified about what is going on here.  Maybe there is a bug in
ethtool, but I'm not seeing it.

Ben.

> >  		}
> >  
> >  		i += ret;
-- 
Ben Hutchings
The two most common things in the universe are hydrogen and stupidity.

Back to linux.debian.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Bug#960702: ethtool -m values change when output is redirected "Yannis Aribaud" <bugs@d6bell.net> - 2020-05-15 17:30 +0200
  Bug#960702: ethtool -m values change when output is redirected Bjørn Mork <bjorn@mork.no> - 2020-05-15 19:00 +0200
    Bug#960702: ethtool -m values change when output is redirected "Yannis Aribaud" <bugs@d6bell.net> - 2020-05-15 19:40 +0200
  Bug#960702: ethtool -m values change when output is redirected Ben Hutchings <ben@decadent.org.uk> - 2020-05-16 01:40 +0200
    Bug#960702: ethtool -m values change when output is redirected "Yannis Aribaud" <bugs@d6bell.net> - 2020-05-17 17:00 +0200
      Bug#960702: ethtool -m values change when output is redirected Ben Hutchings <ben@decadent.org.uk> - 2020-05-17 18:10 +0200
        Bug#960702: ethtool -m values change when output is redirected "Yannis Aribaud" <bugs@d6bell.net> - 2020-05-18 12:10 +0200
  Processed: Re: Bug#960702: ethtool -m values change when output  is redirected "Debian Bug Tracking System" <owner@bugs.debian.org> - 2020-05-16 01:40 +0200
  Processed: Re: Bug#960702: ethtool -m values change when output  is redirected "Debian Bug Tracking System" <owner@bugs.debian.org> - 2020-05-17 18:10 +0200
  Bug#960702: [PATCH net] mlx4: Fix information leak on failure to read module EEPROM Ben Hutchings <ben@decadent.org.uk> - 2020-05-17 19:30 +0200
  Bug#960702: [PATCH net] mlx4: Fix information leak on failure to read module EEPROM Ben Hutchings <ben@decadent.org.uk> - 2020-05-18 20:30 +0200

csiph-web