i2c device driver bugs

i2c device driver bugs

[Shaun Colley]

Hi list,

I wanted to discuss this a little, so I sent the bug
reports here.  This will avoid me writing a complete
bug report, especially if a bug doesn't exist.  I'd
like your opinions on these bugs.



drivers/i2c/i2c-dev.c
i2cdev_ioctl()
Integer overflow
-----------------------

There seems to be a possible integer overflow, which
can come into play when allocating memory.  See below:

--
case I2C_RDWR:
                if (copy_from_user(&rdwr_arg,
                                   (struct
i2c_rdwr_ioctl_data *)arg,
                                   sizeof(rdwr_arg)))
                        return -EFAULT;

                rdwr_pa = (struct i2c_msg *)
                        kmalloc(rdwr_arg.nmsgs *
sizeof(struct i2c_msg),
                        GFP_KERNEL);

                if (rdwr_pa == NULL) return -ENOMEM;

                res = 0;
                for( i=0; i<rdwr_arg.nmsgs; i++ )
                {

[...]
--

As the code shows, the problem exists when parsing the
I2C_RDWR ioctl option.  It seems like an integer
overflow could occur in the below line:

kmalloc(rdwr_arg.nmsgs * sizeof(struct i2c_msg),
                        GFP_KERNEL);

If rdwr_arg_nmsgs held a number which is not
representable when multiplied by sizeof(struct
i2c_msg), an integer overflow could occur.  Since
rdwr_arg.nmsgs is user-supplied, this could warrant a
problem, especially since the integer overflow occurs
during the allocation in memory.

As far as I can tell, the for() loop following the
memory allocation could cause for data to be written
past the allocated memory, since the integer overflow
is likely to have caused too little memory to have
been allocated -- this could warrant a security
problem, if a bug exists as I've interpreted.


drivers/i2c/i2c-core.c
i2cproc_bus_read()
Integer overflow
-----------------------

There is a possible integer overflow problem when
allocating a chunk of memory:

[...]

if (count > 4096)
                return -EINVAL;

[...]

/* We need a bit of slack in the kernel buffer; this
makes the
                   sprintf safe. */
                        if (! (kbuf = kmalloc(count +
80,GFP_KERNEL)))
                                return -ENOMEM;

[...]

---

Although checks are made to ensure that the 'count'
variable doesn't exceed 4096, no checks are made for
negative numbers.  Since kmalloc() takes it count
argument as an unsigned int, a negative number would
be represented as a very large number.  Then, 80 is
added to this number to allocate a would-be large
chunk of memory, but by adding 80, an integer overflow
can occur.  For example, if -1 was passed as count,
kmalloc() would interpret the number as 0xffffffff +
80, which would definitely cause an integer overflow. 
This could cause too little memory to be allocated --
this would cause a problem, since the following
sprintf() calls will almost definately write past the
allocated memory.  Again, this might be a security
issue.


Are any of these bugs likely to be an issue?  I'd like
to hear your comments please :)

(please CC me, I'm not subscribed to the list)




Thank you for your time.
Shaun.





	
	
		
___________________________________________________________ALL-NEW Yahoo! Messenger - sooooo many all-new ways to express yourself http://uk.messenger.yahoo.com

RE: i2c device driver bugs

[Shaun Colley]

Hehe, forgot to specify which kernel source I read
this from.  Well, the first issue seems to be present
in all of the 2.4 kernels, and 2.5 -- I haven't
checked 2.6 yet.

The second bug seems to be fixed in 2.4.9 (and maybe
lower) and 2.5, and probably 2.6, therefore.


If anymore info is needed, let me know.



Thank you for your time.
Shaun.


	
	
		
___________________________________________________________ALL-NEW Yahoo! Messenger - sooooo many all-new ways to express yourself http://uk.messenger.yahoo.com

Re: i2c device driver bugs

[Greg KH]

On Sun, Jun 13, 2004 at 07:41:16PM +0100, Shaun Colley wrote:
> Hehe, forgot to specify which kernel source I read
> this from.  Well, the first issue seems to be present
> in all of the 2.4 kernels, and 2.5 -- I haven't
> checked 2.6 yet.

Please let us know exactly what kernel version you see this in.  It
looks to me that it is fixed in the latest 2.4 and 2.6 versions.  If you
do not think so, please let us know.

thanks,

greg k-h

Re: i2c device driver bugs

[Shaun Colley]

Hi Greg,

> Please let us know exactly what kernel version you
> see this in.  It
> looks to me that it is fixed in the latest 2.4 and
> 2.6 versions.  If you
> do not think so, please let us know.

I was actually looking at a fairly old version of the
source tree (2.4.19, 2.4.20) -- it appears that a
quick fix fixed this vulnerability in 2.4.21:

http://lxr.linux.no/diff/drivers/i2c/i2c-dev.c?diffval=2.4.21;diffvar=v

If you scroll down a bit, you should see:

---
if (rdwr_arg.nmsgs > 42)
          return -EINVAL;
---

It looks like a quick sanity check was added in the
'I2C_RDWR' option, to fix the issue.

I'm downloading the 2.4.21 patch to check if the
fixing of this was recorded, or whether it was
silently fixed (looks like it was).

Confirmed.  2.4.21 fixed the bug:

---
[shaun@cpc2-mars1-3-0-cust191 shaun]$ grep -n -A 10
"sent at once" * 
patch-2.4.21:162276:+		 * be sent at once */
patch-2.4.21-162277-+		if (rdwr_arg.nmsgs > 42)
patch-2.4.21-162278-+			return -EINVAL;
patch-2.4.21-162279-+		
patch-2.4.21-162280- 		rdwr_pa = (struct i2c_msg *)
patch-2.4.21-162281- 			kmalloc(rdwr_arg.nmsgs *
sizeof(struct i2c_msg), 
patch-2.4.21-162282- 			GFP_KERNEL);
patch-2.4.21-162283-@@ -270,6 +275,11 @@
patch-2.4.21-162284- 			        res = -EFAULT;
patch-2.4.21-162285- 				break;
patch-2.4.21-162286- 			}
[shaun@cpc2-mars1-3-0-cust191 shaun]$ 
---


It's also fixed in all versions of 2.6...

However, the vulnerbility seems to still be present in
2.5 -- latest version.  

So, to sum it up:

- Not present in 2.2, because the driver wasn't
implemented as fully as it is now.
- Present in 2.4 versions 2.4.20 and below.
- Present in 2.5
- Not present in 2.6



As for the second bug, it's fixed in 2.4.9, 2.5, and
2.6.



Thank you for your time.
Shaun.



	
	
		
___________________________________________________________ALL-NEW Yahoo! Messenger - sooooo many all-new ways to express yourself http://uk.messenger.yahoo.com

Re: i2c device driver bugs

[Greg KH]

On Tue, Jun 15, 2004 at 04:39:20PM +0100, Shaun Colley wrote:
> Hi Greg,
> 
> > Please let us know exactly what kernel version you
> > see this in.  It
> > looks to me that it is fixed in the latest 2.4 and
> > 2.6 versions.  If you
> > do not think so, please let us know.
> 
> I was actually looking at a fairly old version of the
> source tree (2.4.19, 2.4.20) -- it appears that a
> quick fix fixed this vulnerability in 2.4.21:
> 
> http://lxr.linux.no/diff/drivers/i2c/i2c-dev.c?diffval=2.4.21;diffvar=v
> 
> If you scroll down a bit, you should see:
> 
> ---
> if (rdwr_arg.nmsgs > 42)
>           return -EINVAL;
> ---
> 
> It looks like a quick sanity check was added in the
> 'I2C_RDWR' option, to fix the issue.
> 
> I'm downloading the 2.4.21 patch to check if the
> fixing of this was recorded, or whether it was
> silently fixed (looks like it was).
> 
> Confirmed.  2.4.21 fixed the bug:

What do you mean "silent"?  I got fixed 15 months ago with the following
changeset:
	http://linux.bkbits.net:8080/linux-2.4/diffs/drivers/i2c/i2c-dev.c@1.8

It was then fixed even better with the following change:
	http://linux.bkbits.net:8080/linux-2.4/diffs/drivers/i2c/i2c-dev.c@1.9
almost a whole year ago.

> It's also fixed in all versions of 2.6...
> 
> However, the vulnerbility seems to still be present in
> 2.5 -- latest version.  

Heh, 2.5 development is dead, no one uses that kernel, just like no one
uses the most recent 2.3 kernel tree.

> So, to sum it up:
> 
> - Not present in 2.2, because the driver wasn't
> implemented as fully as it is now.
> - Present in 2.4 versions 2.4.20 and below.
> - Present in 2.5
> - Not present in 2.6

Yes, this was a security issue a year ago, but has been fixed since
then.  Vendors have released kernels that fix this issue for their 2.4
kernels.  If not, I suggest you contact your vendor.

thanks again,

greg k-h

Re: i2c device driver bugs

[Shaun Colley]

Okay, thanks for the info greg.

> Yes, this was a security issue a year ago, but has
> been fixed since
> then.  Vendors have released kernels that fix this
> issue for their 2.4
> kernels.  If not, I suggest you contact your vendor.

What I meant by silent was, did the issue actually get
mentioned in any of the distro vendor's advisories?



Thank you for your time.
Shaun.


	
	
		
___________________________________________________________ALL-NEW Yahoo! Messenger - sooooo many all-new ways to express yourself http://uk.messenger.yahoo.com

Re: i2c device driver bugs

[Greg KH]

On Tue, Jun 15, 2004 at 05:32:44PM +0100, Shaun Colley wrote:
> Okay, thanks for the info greg.
> 
> > Yes, this was a security issue a year ago, but has
> > been fixed since
> > then.  Vendors have released kernels that fix this
> > issue for their 2.4
> > kernels.  If not, I suggest you contact your vendor.
> 
> What I meant by silent was, did the issue actually get
> mentioned in any of the distro vendor's advisories?

I did see it be mentioned in a few.  But it really isn't that big of a
problem, as all distros seemed to have their /dev/i2c* nodes set to:
	$ ls -l /dev/i2c*
	crw-------  1 root root 89, 0 Feb 23 13:02 /dev/i2c0
	crw-------  1 root root 89, 0 Feb 23 13:02 /dev/i2c-0
	crw-------  1 root root 89, 1 Feb 23 13:02 /dev/i2c1
	crw-------  1 root root 89, 1 Feb 23 13:02 /dev/i2c-1

Which prevents any normal user from exploiting this issue.

thanks,

greg k-h

Re: i2c device driver bugs

[Shaun Colley]

Well, I can't see it mentioned anywhere, but perhaps
you're right.  Anyway, thanks for the info.  Although
this isn't so much of a big deal (because of the fixes
and mitagating factor), I think they would be
exploitable in certain conditions, maybe...

Last word: please could you point me to where you saw
an announcement for these bugs in a security advisory?



Thank you for your time.
Shaun.


	
	
		
___________________________________________________________ALL-NEW Yahoo! Messenger - sooooo many all-new ways to express yourself http://uk.messenger.yahoo.com

Re: i2c device driver bugs

[Greg KH]

On Tue, Jun 15, 2004 at 06:09:12PM +0100, Shaun Colley wrote:
> Well, I can't see it mentioned anywhere, but perhaps
> you're right.  Anyway, thanks for the info.  Although
> this isn't so much of a big deal (because of the fixes
> and mitagating factor), I think they would be
> exploitable in certain conditions, maybe...
> 
> Last word: please could you point me to where you saw
> an announcement for these bugs in a security advisory?

Don't exactly remember, it was over a year ago :)
I think it was in some Red Hat kernel update, but again, can't recall.

thanks,

greg k-h