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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 01 Apr 2008 21:21:19 +0200 From: Eric Dumazet <dada1@...mosbay.com> To: Evgeniy Polyakov <johnpol@....mipt.ru> CC: David Miller <davem@...emloft.net>, axboe@...nel.dk, netdev@...r.kernel.org Subject: Re: Fix for the fundamental network/block layer race in sendfile(). Evgeniy Polyakov a écrit : > On Tue, Apr 01, 2008 at 09:47:59PM +0400, Evgeniy Polyakov (johnpol@....mipt.ru) wrote: >>> 1) Is out_file garanted to be a socket ? >>> 2) SO_PRIVATE_CALLBACK is defined to be 37 later on your patch. Is ORing 37 >>> to sk_flags really working ??? >>> 3) So, once a socket has been used for a sendfile(), all subsequent >>> sendmsg() will call skb_splice_destructor ? (I see no clearing of >>> SO_PRIVATE_CALLBACK) >>> This will probably crash. >>> 4) god_blessed_us name ... Oh I got it, its April the first !!!! > > Addressed all above (2 and 3 actually). > The formed by using correct socket flag (set_bit/clear_bit, like in a > real life), the latter by clearing bit on sendfile() exit and to fix > exit/clear race each spliced page also setups lru.prev to itself, which > is checked in destructor. > Name was not changed, does it matter? > > Patch still works ok :) > > Thanks Eric for pointing that bugs. > > Signed-off-as-usual. > > diff --git a/fs/read_write.c b/fs/read_write.c > index 49a9871..fda55f6 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -16,6 +16,7 @@ > #include <linux/syscalls.h> > #include <linux/pagemap.h> > #include <linux/splice.h> > +#include <net/sock.h> > #include "read_write.h" > > #include <asm/uaccess.h> > @@ -703,6 +704,8 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, > loff_t pos; > ssize_t retval; > int fput_needed_in, fput_needed_out, fl; > + struct sock *sk; > + struct socket *sock; > > /* > * Get input file, and verify that it is ok.. > @@ -762,6 +765,12 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, > count = max - pos; > } > > + sock = out_file->private_data; > + sk = sock->sk; > + > + sk->sk_user_data = &skb_splice_destructor; > + set_bit(SOCK_PRIVATE_CALLBACK, &sock->flags); > + > fl = 0; > #if 0 > /* > @@ -775,6 +784,8 @@ static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos, > #endif > retval = do_splice_direct(in_file, ppos, out_file, count, fl); > > + clear_bit(SOCK_PRIVATE_CALLBACK, &sock->flags); I see no socket locking, so multiple threads could use sendfile() & sendmsg() on same socket, and crash kernel... -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists