Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH -mm] swsusp: userland interface (rev 2) | Date | Wed, 25 Jan 2006 13:29:55 +0100 |
| |
Hi,
On Wednesday, 25 January 2006 13:18, Pavel Machek wrote: > > > > + case SNAPSHOT_ATOMIC_RESTORE: > > > > + if (data->mode != O_WRONLY || !data->frozen || > > > > + !snapshot_image_loaded(&data->handle)) { > > > > + error = -EPERM; > > > > + break; > > > > + } > > > > + down(&pm_sem); > > > > + pm_prepare_console(); > > > > + error = device_suspend(PMSG_FREEZE); > > > > + if (!error) { > > > > + mb(); > > > > + error = swsusp_resume(); > > > > + device_resume(); > > > > + } > > > > > > whee, what does the mystery barrier do? It'd be nice to comment this > > > (please always comment open-coded barriers). > > > > Pavel should know. ;-) > > Pavel does not known. That memory barrier should be part of assembly > parts, anyway, and AFAIK it is. Should be safe to kill.
OK
> > > > + case SNAPSHOT_GET_SWAP_PAGE: > > > > + if (!access_ok(VERIFY_WRITE, (unsigned long __user *)arg, _IOC_SIZE(cmd))) { > > > > + error = -EINVAL; > > > > + break; > > > > + } > > > > > > Why do we need an access_ok() here? > > > > Because we use __put_user() down the road? > > > > The problem is if the address is wrong we should not try to call > > alloc_swap_page() at all. If we did, we wouldn't be able to return the result > > and we would leak a swap page. > > I think you need to watch for failing put_user and free the page at > that point. Anything else is racy as __put_user() may fail.
The page will be freed anyway when the device is closed (I was wrong saying it would be "leaked"), so I think I'll just use put_user().
Greetings, Rafael - 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/
| |