Messages in this thread | | | Date | Wed, 25 Jan 2006 13:18:48 +0100 | From | Pavel Machek <> | Subject | Re: [PATCH -mm] swsusp: userland interface (rev 2) |
| |
Hi!
> > > + 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.
> > > + 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.
Pavel -- Thanks, Sharp! - 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/
| |