lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 28 Dec 2012 15:12:49 +0100 From: Oleg Nesterov <oleg@...hat.com> To: Andrey Wagin <avagin@...il.com> Cc: Andrew Vagin <avagin@...allels.com>, Pavel Emelyanov <xemul@...allels.com>, David Howells <dhowells@...hat.com>, linux-kernel@...r.kernel.org, criu@...nvz.org, Cyrill Gorcunov <gorcunov@...nvz.org>, Alexander Viro <viro@...iv.linux.org.uk>, linux-fsdevel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>, "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>, Michael Kerrisk <mtk.manpages@...il.com> Subject: Re: [CRIU] [PATCH 1/4] signalfd: add ability to return siginfo in a raw format On 12/27, Andrey Wagin wrote: > > 2012/12/27 Oleg Nesterov <oleg@...hat.com>: > > On 12/27, Andrey Wagin wrote: > >> > >> On Wed, Dec 26, 2012 at 05:31:12PM +0100, Oleg Nesterov wrote: > >> > > >> > So I think we should not use llseek. But, probably we can rely on pread() ? > >> > This way we can avoid the problem above, and this looks even simpler. > >> > >> Yes. It is a good idea. A new patch is attached to this email. I > >> implemented pread for signalfd and fixed all your previous comments. > >> > ... > > I think it would be better to pass ppos, not *ppos, because ... > > > >> + if (*ppos) > >> + (*ppos)++; > > > > in this case we can update *ppos in signalfd_peek() and simplify the > > code a bit. > > > > Compared to the previous version it is "safe" to change *ppos even if > > copy_to_user() fails, *ppos will be "lost" anyway after we return. > > Yes. But ppos is updated, because pread() reads all siginfo-s from a queue, > which fit in a buffer starting with the initial ppos. Can't understand. And I guess you misunderstood. I meant, we can update *ppos in signalfd_peek() _and_ we can do this unconditionally even if copy_to_user() failed. > >> @@ -321,6 +372,7 @@ SYSCALL_DEFINE4(signalfd4, int, ufd, sigset_t __user *, user_mask, > >> } > >> > >> file->f_flags |= flags & SFD_RAW; > >> + file->f_mode |= FMODE_PREAD; > >> > >> fd_install(ufd, file); > > > > Hmm. Looks like it is based on other patches I didnt see... > > > > But I don't understand how FMODE_PREAD connects to this patch, we need > > this flag set even for regular sys_read() ??? > > It doesn't need for sys_read(), but this patch is about pread() and > sys_pread() checks it. > > SYSCALL_DEFINE(pread64)(unsigned int fd, char __user *buf, > size_t count, loff_t pos) > { > .... > if (f.file) { > ret = -ESPIPE; > if (f.file->f_mode & FMODE_PREAD) > ret = vfs_read(f.file, buf, count, &pos); And sys_read() checks it too, that was my point. So either this code was already broken before this patch, or FMODE_PREAD is already set and you do not need this chunk. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists