On Thursday, December 29, 2011, Ben Hutchings wrote:
> On Tue, 2011-12-27 at 21:33 +0100, Ben Hutchings wrote:
> > This allows uswsusp built for i386 to run on an x86_64 kernel (tested
> > with Debian package version 1.0+20110509-2).
> 
> Now I wonder whether all this code was necessary.
> 
> [...]
> > +#ifdef CONFIG_COMPAT
> > +
> > +struct compat_resume_swap_area {
> > +   compat_loff_t offset;
> > +   u32 dev;
> > +} __packed;
> [...]
> 
> Is there actually any case where this doesn't have compatible layout
> with struct resume_swap_area?  It may have lower alignment, but
> misalignment is dealt with automatically when copying in/out of
> userland.

Quite frankly, I'm not 100% sure.

> If the structure size was variable then the ioctl number would be as
> well, and commit cf007e3526a785a95a738d5a8fba44f1f4fe33e0 is supposed to
> have removed the last ioctls that had that problem.
> 
> So maybe the compat function can be as simple as:
> 
> static long
> snapshot_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> {
>       BUILD_BUG_ON(sizeof(loff_t) != sizeof(compat_loff_t));
>       BUILD_BUG_ON(sizeof(struct resume_swap_area) !=
>                    sizeof(struct compat_resume_swap_area));
> 

Well, I'd prefer to fail the ioctl in the above situations instead of crashing
the kernel.

>       switch (cmd) {
>       case SNAPSHOT_SET_SWAP_AREA:
>       case SNAPSHOT_GET_IMAGE_SIZE:
>       case SNAPSHOT_CREATE_IMAGE:
>       case SNAPSHOT_AVAIL_SWAP_SIZE:
>       case SNAPSHOT_ALLOC_SWAP_PAGE:
>               return snapshot_ioctl(file, cmd,
>                                     (unsigned long) compat_ptr(arg));
> 
>       default:
>               return snapshot_ioctl(file, cmd, arg);
>       }
> }

Does it acutally work?

Rafael



-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/201112292333.31303....@sisk.pl

Reply via email to