re: [SCSI] compat_ioct: fix bsg SG_IO

re: [SCSI] compat_ioct: fix bsg SG_IO

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]

84eb8fb42c120 "[SCSI] compat_ioct: fix bsg SG_IO" introduces an
uninitialized variable use.

   278  static int sg_ioctl_trans(unsigned int fd, unsigned int cmd,
   279                          sg_io_hdr32_t __user *sgio32)
   280  {
   281          sg_io_hdr_t __user *sgio;
   282          u16 iovec_count;
   283          u32 data;
   284          void __user *dxferp;
   285          int err;
   286          int interface_id;
   287  
   288          if (get_user(interface_id, &sgio32->interface_id))
                                            ^^^^^^
sgio32 is unitialized here.  Unfortunately Gcc doesn't warn about it.

   289                  return -EFAULT;
   290          if (interface_id != 'S')
   291                  return sys_ioctl(fd, cmd, (unsigned long)sgio32);

regards,
dan carpenter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

re: [SCSI] compat_ioct: fix bsg SG_IO

[James Bottomley]

On Wed, 2011-11-02 at 11:16 +0300, Dan Carpenter wrote:
> 84eb8fb42c120 "[SCSI] compat_ioct: fix bsg SG_IO" introduces an
> uninitialized variable use.
> 
>    278  static int sg_ioctl_trans(unsigned int fd, unsigned int cmd,
>    279                          sg_io_hdr32_t __user *sgio32)
>    280  {
>    281          sg_io_hdr_t __user *sgio;
>    282          u16 iovec_count;
>    283          u32 data;
>    284          void __user *dxferp;
>    285          int err;
>    286          int interface_id;
>    287  
>    288          if (get_user(interface_id, &sgio32->interface_id))
>                                             ^^^^^^
> sgio32 is unitialized here.  Unfortunately Gcc doesn't warn about it.

I don't quite understand what makes you think that: it's passed in as an
argument to the function.  It's a pointer to the userspace 32 bit
representation of the structure.  The use logic is a slightly convoluted
way of saying we only understand the 'S' header but we're going to let
the real ioctl routine say what the error is if it's not type 'S'.

James

Re: [SCSI] compat_ioct: fix bsg SG_IO

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

On Wed, Nov 02, 2011 at 12:58:58PM +0400, James Bottomley wrote:
> On Wed, 2011-11-02 at 11:16 +0300, Dan Carpenter wrote:
> > 84eb8fb42c120 "[SCSI] compat_ioct: fix bsg SG_IO" introduces an
> > uninitialized variable use.
> > 
> >    278  static int sg_ioctl_trans(unsigned int fd, unsigned int cmd,
> >    279                          sg_io_hdr32_t __user *sgio32)
> >    280  {
> >    281          sg_io_hdr_t __user *sgio;
> >    282          u16 iovec_count;
> >    283          u32 data;
> >    284          void __user *dxferp;
> >    285          int err;
> >    286          int interface_id;
> >    287  
> >    288          if (get_user(interface_id, &sgio32->interface_id))
> >                                             ^^^^^^
> > sgio32 is unitialized here.  Unfortunately Gcc doesn't warn about it.
> 
> I don't quite understand what makes you think that: it's passed in as an
> argument to the function.  It's a pointer to the userspace 32 bit
> representation of the structure.  The use logic is a slightly convoluted
> way of saying we only understand the 'S' header but we're going to let
> the real ioctl routine say what the error is if it's not type 'S'.

Uh...  This is embarrassing.  I got confused between sgio and sgio32.

Sorry for this.

regards,
dan carpenter

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]