FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Debian > Debian Kernel

 
 
LinkBack Thread Tools
 
Old 12-27-2011, 07:33 PM
Ben Hutchings
 
Default Bug#502816: snapshot: Implement compat_ioctl

This allows uswsusp built for i386 to run on an x86_64 kernel (tested
with Debian package version 1.0+20110509-2).

References: http://bugs.debian.org/502816
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
kernel/power/user.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ +
1 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 6d8f535..d86e5a7 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -21,6 +21,7 @@
#include <linux/swapops.h>
#include <linux/pm.h>
#include <linux/fs.h>
+#include <linux/compat.h>
#include <linux/console.h>
#include <linux/cpu.h>
#include <linux/freezer.h>
@@ -464,6 +465,66 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
return error;
}

+#ifdef CONFIG_COMPAT
+
+struct compat_resume_swap_area {
+ compat_loff_t offset;
+ u32 dev;
+} __packed;
+
+static long
+snapshot_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ BUILD_BUG_ON(sizeof(loff_t) != sizeof(compat_loff_t));
+
+ switch (cmd) {
+ case SNAPSHOT_GET_IMAGE_SIZE:
+ case SNAPSHOT_AVAIL_SWAP_SIZE:
+ case SNAPSHOT_ALLOC_SWAP_PAGE: {
+ compat_loff_t __user *uoffset = compat_ptr(arg);
+ loff_t offset;
+ mm_segment_t old_fs;
+ int err;
+
+ old_fs = get_fs();
+ set_fs(KERNEL_DS);
+ err = snapshot_ioctl(file, cmd, (unsigned long) &offset);
+ set_fs(old_fs);
+ if (!err && put_user(offset, uoffset))
+ err = -EFAULT;
+ return err;
+ }
+
+ case SNAPSHOT_CREATE_IMAGE:
+ return snapshot_ioctl(file, cmd,
+ (unsigned long) compat_ptr(arg));
+
+ case SNAPSHOT_SET_SWAP_AREA: {
+ struct compat_resume_swap_area __user *u_swap_area =
+ compat_ptr(arg);
+ struct resume_swap_area swap_area;
+ mm_segment_t old_fs;
+ int err;
+
+ err = get_user(swap_area.offset, &u_swap_area->offset);
+ err |= get_user(swap_area.dev, &u_swap_area->dev);
+ if (err)
+ return -EFAULT;
+ old_fs = get_fs();
+ set_fs(KERNEL_DS);
+ err = snapshot_ioctl(file, SNAPSHOT_SET_SWAP_AREA,
+ (unsigned long) &swap_area);
+ set_fs(old_fs);
+ return err;
+ }
+
+ default:
+ return snapshot_ioctl(file, cmd, arg);
+ }
+}
+
+#endif /* CONFIG_COMPAT */
+
static const struct file_operations snapshot_fops = {
.open = snapshot_open,
.release = snapshot_release,
@@ -471,6 +532,9 @@ static const struct file_operations snapshot_fops = {
.write = snapshot_write,
.llseek = no_llseek,
.unlocked_ioctl = snapshot_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = snapshot_compat_ioctl,
+#endif
};

static struct miscdevice snapshot_device = {
--
1.7.7.3





--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 1325018001.2327.13.camel@deadeye">http://lists.debian.org/1325018001.2327.13.camel@deadeye
 
Old 12-27-2011, 09:03 PM
"Rafael J. Wysocki"
 
Default Bug#502816: snapshot: Implement compat_ioctl

On Tuesday, December 27, 2011, 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).
>
> References: http://bugs.debian.org/502816
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

Applied to linux-pm/linux-next.

Thanks,
Rafael


> ---
> kernel/power/user.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ +
> 1 files changed, 64 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index 6d8f535..d86e5a7 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -21,6 +21,7 @@
> #include <linux/swapops.h>
> #include <linux/pm.h>
> #include <linux/fs.h>
> +#include <linux/compat.h>
> #include <linux/console.h>
> #include <linux/cpu.h>
> #include <linux/freezer.h>
> @@ -464,6 +465,66 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
> return error;
> }
>
> +#ifdef CONFIG_COMPAT
> +
> +struct compat_resume_swap_area {
> + compat_loff_t offset;
> + u32 dev;
> +} __packed;
> +
> +static long
> +snapshot_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + BUILD_BUG_ON(sizeof(loff_t) != sizeof(compat_loff_t));
> +
> + switch (cmd) {
> + case SNAPSHOT_GET_IMAGE_SIZE:
> + case SNAPSHOT_AVAIL_SWAP_SIZE:
> + case SNAPSHOT_ALLOC_SWAP_PAGE: {
> + compat_loff_t __user *uoffset = compat_ptr(arg);
> + loff_t offset;
> + mm_segment_t old_fs;
> + int err;
> +
> + old_fs = get_fs();
> + set_fs(KERNEL_DS);
> + err = snapshot_ioctl(file, cmd, (unsigned long) &offset);
> + set_fs(old_fs);
> + if (!err && put_user(offset, uoffset))
> + err = -EFAULT;
> + return err;
> + }
> +
> + case SNAPSHOT_CREATE_IMAGE:
> + return snapshot_ioctl(file, cmd,
> + (unsigned long) compat_ptr(arg));
> +
> + case SNAPSHOT_SET_SWAP_AREA: {
> + struct compat_resume_swap_area __user *u_swap_area =
> + compat_ptr(arg);
> + struct resume_swap_area swap_area;
> + mm_segment_t old_fs;
> + int err;
> +
> + err = get_user(swap_area.offset, &u_swap_area->offset);
> + err |= get_user(swap_area.dev, &u_swap_area->dev);
> + if (err)
> + return -EFAULT;
> + old_fs = get_fs();
> + set_fs(KERNEL_DS);
> + err = snapshot_ioctl(file, SNAPSHOT_SET_SWAP_AREA,
> + (unsigned long) &swap_area);
> + set_fs(old_fs);
> + return err;
> + }
> +
> + default:
> + return snapshot_ioctl(file, cmd, arg);
> + }
> +}
> +
> +#endif /* CONFIG_COMPAT */
> +
> static const struct file_operations snapshot_fops = {
> .open = snapshot_open,
> .release = snapshot_release,
> @@ -471,6 +532,9 @@ static const struct file_operations snapshot_fops = {
> .write = snapshot_write,
> .llseek = no_llseek,
> .unlocked_ioctl = snapshot_ioctl,
> +#ifdef CONFIG_COMPAT
> + .compat_ioctl = snapshot_compat_ioctl,
> +#endif
> };
>
> static struct miscdevice snapshot_device = {
>




--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 201112272303.50335.rjw@sisk.pl">http://lists.debian.org/201112272303.50335.rjw@sisk.pl
 
Old 12-29-2011, 12:27 PM
Ben Hutchings
 
Default Bug#502816: snapshot: Implement compat_ioctl

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.

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));

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);
}
}

Ben.

--
Ben Hutchings
Design a system any fool can use, and only a fool will want to use it.
 
Old 12-29-2011, 09:33 PM
"Rafael J. Wysocki"
 
Default Bug#502816: snapshot: Implement compat_ioctl

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-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 201112292333.31303.rjw@sisk.pl">http://lists.debian.org/201112292333.31303.rjw@sisk.pl
 
Old 12-29-2011, 10:42 PM
Ben Hutchings
 
Default Bug#502816: snapshot: Implement compat_ioctl

On Thu, 2011-12-29 at 23:33 +0100, Rafael J. Wysocki wrote:
> 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 it wasn't for the '__packed' in the declaration of struct
resume_swap_area, there would be 4 trailing bytes of padding on 64-bit
architectures and none on most 32-bit architectures. But __packed
should inhibit that padding.

I was apparently mistaken about put_user(); that would need to be
changed to put_user_unaligned() if we want to handle compat_loff_t
without a wrapper.

> > 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.

This is a compile-time check...

> > 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?

I haven't tested, but in any case testing on x86 wouldn't prove that
this works for any other 32/64-bit architecture pair.

Ben.

--
Ben Hutchings
Design a system any fool can use, and only a fool will want to use it.
 
Old 12-29-2011, 10:52 PM
"Rafael J. Wysocki"
 
Default Bug#502816: snapshot: Implement compat_ioctl

On Friday, December 30, 2011, Ben Hutchings wrote:
> On Thu, 2011-12-29 at 23:33 +0100, Rafael J. Wysocki wrote:
> > 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 it wasn't for the '__packed' in the declaration of struct
> resume_swap_area, there would be 4 trailing bytes of padding on 64-bit
> architectures and none on most 32-bit architectures. But __packed
> should inhibit that padding.
>
> I was apparently mistaken about put_user(); that would need to be
> changed to put_user_unaligned() if we want to handle compat_loff_t
> without a wrapper.
>
> > > 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.
>
> This is a compile-time check...

That's correct, sorry.

> > > 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?
>
> I haven't tested, but in any case testing on x86 wouldn't prove that
> this works for any other 32/64-bit architecture pair.

Well, if it doesn't work on x86, it will prove something. :-)

Thanks,
Rafael



--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 201112300052.11690.rjw@sisk.pl">http://lists.debian.org/201112300052.11690.rjw@sisk.pl
 

Thread Tools




All times are GMT. The time now is 10:07 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org