On Tue, Jan 7, 2020 at 4:17 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> On Tue, Jan 07, 2020 at 04:03:12PM +0100, Arnd Bergmann wrote:
> > On Tue, Jan 7, 2020 at 9:49 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> > >
> > > Hello Arnd Bergmann,
> > >
> > > The patch f3ee6e63a9df: "compat_ioctl: move CDROM_SEND_PACKET
> > > handling into scsi" from Nov 28, 2019, leads to the following static
> > > checker warning:
> > >
> > > block/scsi_ioctl.c:703 scsi_put_cdrom_generic_arg()
> > > warn: check that 'cgc32' doesn't leak information (struct has a hole after 'data_direction')
> > >
> > > block/scsi_ioctl.c
> > > 686 static int scsi_put_cdrom_generic_arg(const struct cdrom_generic_command *cgc,
> > > 687 void __user *arg)
> > > 688 {
> > > 689 #ifdef CONFIG_COMPAT
> > > 690 if (in_compat_syscall()) {
> > > 691 struct compat_cdrom_generic_command cgc32 = {
> > > 692 .buffer = (uintptr_t)(cgc->buffer),
> > > 693 .buflen = cgc->buflen,
> > > 694 .stat = cgc->stat,
> > > 695 .sense = (uintptr_t)(cgc->sense),
> > > 696 .data_direction = cgc->data_direction,
> > > 697 .quiet = cgc->quiet,
> > > 698 .timeout = cgc->timeout,
> > > 699 .reserved[0] = (uintptr_t)(cgc->reserved[0]),
> > > 700 };
> > >
> > > It's possible that initializations like this don't clear out the struct
> > > hole but I haven't seen a compiler which is affected. So maybe it's
> > > fine?
> >
> > I thlought we already rely on this to initialize the entire structure, but
> > trying out a test case shows that it does happen:
>
> There aren't that many cases where we rely on it to happen. Under 20
> so far as Smatch can detect. I'm not really certain what the correct
> approach is to deal with them... I think they pretty much all work
> fine with existing compilers.
After looking a bit more into this, I'm now fairly convinced this is a
real problem. On gcc, this is prevented from causing too much harm
by the structleak plugin, but that is not always enabled.
I'll send fixes for the ones I recently introduced. Can you send me a list
of the other instances that smatch finds? Maybe I can take a look at
those as well.
Arnd
