Re: [PATCH] Fix blktrace setup 32-bit ioctl on 64-bit kernels

[Jens Axboe <jens.axboe@oracle.com>]

On Thu, Oct 04 2007, Arnd Bergmann wrote:
> On Wednesday 03 October 2007, Arnd Bergmann wrote:
> > Jens, I think the best overall solution would be to have a
> > block/compat_ioctl.c file with all the compat handling for block
> > devices moved over from fs/compat_ioctl.c, and done in a nicer way.
> > If you agree, with this approach, I'd volunteer to come up with a
> > patch.
> 
> Sometimes I find it hard to stop myself once I have the idea.
> The patch below moves all block related ioctl conversion out
> of fs/compat_ioctl.c into the compat_blkdev_ioctl() function.
> 
> Is that a direction we should be heading towards? If so, I
> can do some testing and split this big patch into more
> logical units for better review.

This looks a lot better! I don't mind seperating the block bits, when we
get the whole bunch in there. Just seemed overly silly and complicated
to do it for just one ioctl command. When you are happy with this patch,
I'll add it to the pending block stuff for 2.6.24. The blktrace fix
isn't a regression so need not go into 2.6.23.

It seems some bits are missing though (like BLKTRACESETUP32), did you
send the whole thing?

> I also found a few interesting bugs in the process:
> * BLKRASET is both ULONG_IOCTL and COMPATIBLE_IOCTL, but this should
>   be entirely harmless
> * BLKSECTGET writes 2 bytes normally, our compat_ version writes
>   4 bytes!
> * FDSETPRM32, FDDEFPRM32 and FDGETPRM32 are doing potentially dangerous
>   stuff with kernel pointers. sparse actually warns about this, but
>   from what I could see from floppy.c, the kernel always ignores these
>   pointers. I did not check any other floppy drivers implementing the
>   same calls.

Always nice to find bugs when doing cleanups ;-)


-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/