Thread View: gmane.linux.kernel
7 messages
7 total messages
Started by Davide Libenzi
Fri, 16 Sep 2005 16:35
[patch] Fix epoll delayed initialization bug ...
Author: Davide Libenzi
Date: Fri, 16 Sep 2005 16:35
Date: Fri, 16 Sep 2005 16:35
117 lines
3419 bytes
3419 bytes
Al found a potential problem in epoll_create(), where the file->private_data member was set after fd_install(). This is obviously wrong since another thread might do a close() on that fd# before we set the file->private_data member. This goes over 2.6.13 and passes a few basic tests I've done here. Signed-off-by: Davide Libenzi <davidel@xmailserver.org> - Davide diff -Nru linux-2.6.13.vanilla/fs/eventpoll.c linux-2.6.13/fs/eventpoll.c --- linux-2.6.13.vanilla/fs/eventpoll.c 2005-09-16 15:20:46.000000000 -0700 +++ linux-2.6.13/fs/eventpoll.c 2005-09-16 15:21:08.000000000 -0700 @@ -231,8 +231,9 @@ static void ep_poll_safewake_init(struct poll_safewake *psw); static void ep_poll_safewake(struct poll_safewake *psw, wait_queue_head_t *wq); -static int ep_getfd(int *efd, struct inode **einode, struct file **efile); -static int ep_file_init(struct file *file); +static int ep_getfd(int *efd, struct inode **einode, struct file **efile, + struct eventpoll *ep); +static int ep_alloc(struct eventpoll **pep); static void ep_free(struct eventpoll *ep); static struct epitem *ep_find(struct eventpoll *ep, struct file *file, int fd); static void ep_use_epitem(struct epitem *epi); @@ -501,38 +502,37 @@ asmlinkage long sys_epoll_create(int size) { int error, fd; + struct eventpoll *ep; struct inode *inode; struct file *file; DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d)\n", current, size)); - /* Sanity check on the size parameter */ + /* + * Sanity check on the size parameter, and create the internal data + * structure ( "struct eventpoll" ). + */ error = -EINVAL; - if (size <= 0) + if (size <= 0 || (error = ep_alloc(&ep))) goto eexit_1; /* * Creates all the items needed to setup an eventpoll file. That is, * a file structure, and inode and a free file descriptor. */ - error = ep_getfd(&fd, &inode, &file); - if (error) - goto eexit_1; - - /* Setup the file internal data structure ( "struct eventpoll" ) */ - error = ep_file_init(file); + error = ep_getfd(&fd, &inode, &file, ep); if (error) goto eexit_2; - DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d) = %d\n", current, size, fd)); return fd; eexit_2: - sys_close(fd); + ep_free(ep); + kfree(ep); eexit_1: DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d) = %d\n", current, size, error)); @@ -706,7 +706,8 @@ /* * Creates the file descriptor to be used by the epoll interface. */ -static int ep_getfd(int *efd, struct inode **einode, struct file **efile) +static int ep_getfd(int *efd, struct inode **einode, struct file **efile, + struct eventpoll *ep) { struct qstr this; char name[32]; @@ -756,7 +757,7 @@ file->f_op = &eventpoll_fops; file->f_mode = FMODE_READ; file->f_version = 0; - file->private_data = NULL; + file->private_data = ep; /* Install the new setup file into the allocated fd. */ fd_install(fd, file); @@ -777,7 +778,7 @@ } -static int ep_file_init(struct file *file) +static int ep_alloc(struct eventpoll **pep) { struct eventpoll *ep; @@ -792,9 +793,9 @@ INIT_LIST_HEAD(&ep->rdllist); ep->rbr = RB_ROOT; - file->private_data = ep; + *pep = ep; - DNPRINTK(3, (KERN_INFO "[%p] eventpoll: ep_file_init() ep=%p\n", + DNPRINTK(3, (KERN_INFO "[%p] eventpoll: ep_alloc() ep=%p\n", current, ep)); return 0; }
Re: [patch] Fix epoll delayed initialization bug ...
Author: Andrew Morton
Date: Fri, 16 Sep 2005 16:50
Date: Fri, 16 Sep 2005 16:50
22 lines
1002 bytes
1002 bytes
Davide Libenzi <davidel@xmailserver.org> wrote: > > diff -Nru linux-2.6.13.vanilla/fs/eventpoll.c linux-2.6.13/fs/eventpoll.c > --- linux-2.6.13.vanilla/fs/eventpoll.c 2005-09-16 15:20:46.000000000 -0700 > +++ linux-2.6.13/fs/eventpoll.c 2005-09-16 15:21:08.000000000 -0700 > @@ -231,8 +231,9 @@ > > static void ep_poll_safewake_init(struct poll_safewake *psw); > static void ep_poll_safewake(struct poll_safewake *psw, wait_queue_head_t *wq); > -static int ep_getfd(int *efd, struct inode **einode, struct file **efile); > -static int ep_file_init(struct file *file); > +static int ep_getfd(int *efd, struct inode **einode, struct file **efile, > + struct eventpoll *ep); > +static int ep_alloc(struct eventpoll **pep); Sigh. Space-stuffing strikes again. Please resend as an attachment. The number of whitespace-buggered patches which are coming in is just getting out of control lately. Even `patch -l' tossed four rejects, so there may be something else wrong in this one.
Re: [patch] Fix epoll delayed initialization bug ...
Author: Davide Libenzi
Date: Fri, 16 Sep 2005 16:56
Date: Fri, 16 Sep 2005 16:56
113 lines
5931 bytes
5931 bytes
This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323328-1689195616-1126915013=:6125 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed On Fri, 16 Sep 2005, Andrew Morton wrote: > Davide Libenzi <davidel@xmailserver.org> wrote: >> >> diff -Nru linux-2.6.13.vanilla/fs/eventpoll.c linux-2.6.13/fs/eventpoll.c >> --- linux-2.6.13.vanilla/fs/eventpoll.c 2005-09-16 15:20:46.000000000 -0700 >> +++ linux-2.6.13/fs/eventpoll.c 2005-09-16 15:21:08.000000000 -0700 >> @@ -231,8 +231,9 @@ >> >> static void ep_poll_safewake_init(struct poll_safewake *psw); >> static void ep_poll_safewake(struct poll_safewake *psw, wait_queue_head_t *wq); >> -static int ep_getfd(int *efd, struct inode **einode, struct file **efile); >> -static int ep_file_init(struct file *file); >> +static int ep_getfd(int *efd, struct inode **einode, struct file **efile, >> + struct eventpoll *ep); >> +static int ep_alloc(struct eventpoll **pep); > > Sigh. Space-stuffing strikes again. Please resend as an attachment. > > The number of whitespace-buggered patches which are coming in is just > getting out of control lately. > > Even `patch -l' tossed four rejects, so there may be something else wrong > in this one. My side or your side? Anyway, see if the one attached merges cleanly ... - Davide --8323328-1689195616-1126915013=:6125 Content-Type: TEXT/plain; charset=US-ASCII; name=epoll-viromatic-presetup-fix.diff Content-Transfer-Encoding: BASE64 Content-Description: Content-Disposition: attachment; filename=epoll-viromatic-presetup-fix.diff ZGlmZiAtTnJ1IGxpbnV4LTIuNi4xMy52YW5pbGxhL2ZzL2V2ZW50cG9sbC5j IGxpbnV4LTIuNi4xMy9mcy9ldmVudHBvbGwuYw0KLS0tIGxpbnV4LTIuNi4x My52YW5pbGxhL2ZzL2V2ZW50cG9sbC5jCTIwMDUtMDktMTYgMTU6MjA6NDYu MDAwMDAwMDAwIC0wNzAwDQorKysgbGludXgtMi42LjEzL2ZzL2V2ZW50cG9s bC5jCTIwMDUtMDktMTYgMTU6MjE6MDguMDAwMDAwMDAwIC0wNzAwDQpAQCAt MjMxLDggKzIzMSw5IEBADQogDQogc3RhdGljIHZvaWQgZXBfcG9sbF9zYWZl d2FrZV9pbml0KHN0cnVjdCBwb2xsX3NhZmV3YWtlICpwc3cpOw0KIHN0YXRp YyB2b2lkIGVwX3BvbGxfc2FmZXdha2Uoc3RydWN0IHBvbGxfc2FmZXdha2Ug KnBzdywgd2FpdF9xdWV1ZV9oZWFkX3QgKndxKTsNCi1zdGF0aWMgaW50IGVw X2dldGZkKGludCAqZWZkLCBzdHJ1Y3QgaW5vZGUgKiplaW5vZGUsIHN0cnVj dCBmaWxlICoqZWZpbGUpOw0KLXN0YXRpYyBpbnQgZXBfZmlsZV9pbml0KHN0 cnVjdCBmaWxlICpmaWxlKTsNCitzdGF0aWMgaW50IGVwX2dldGZkKGludCAq ZWZkLCBzdHJ1Y3QgaW5vZGUgKiplaW5vZGUsIHN0cnVjdCBmaWxlICoqZWZp bGUsDQorCQkgICAgc3RydWN0IGV2ZW50cG9sbCAqZXApOw0KK3N0YXRpYyBp bnQgZXBfYWxsb2Moc3RydWN0IGV2ZW50cG9sbCAqKnBlcCk7DQogc3RhdGlj IHZvaWQgZXBfZnJlZShzdHJ1Y3QgZXZlbnRwb2xsICplcCk7DQogc3RhdGlj IHN0cnVjdCBlcGl0ZW0gKmVwX2ZpbmQoc3RydWN0IGV2ZW50cG9sbCAqZXAs IHN0cnVjdCBmaWxlICpmaWxlLCBpbnQgZmQpOw0KIHN0YXRpYyB2b2lkIGVw X3VzZV9lcGl0ZW0oc3RydWN0IGVwaXRlbSAqZXBpKTsNCkBAIC01MDEsMzgg KzUwMiwzNyBAQA0KIGFzbWxpbmthZ2UgbG9uZyBzeXNfZXBvbGxfY3JlYXRl KGludCBzaXplKQ0KIHsNCiAJaW50IGVycm9yLCBmZDsNCisJc3RydWN0IGV2 ZW50cG9sbCAqZXA7DQogCXN0cnVjdCBpbm9kZSAqaW5vZGU7DQogCXN0cnVj dCBmaWxlICpmaWxlOw0KIA0KIAlETlBSSU5USygzLCAoS0VSTl9JTkZPICJb JXBdIGV2ZW50cG9sbDogc3lzX2Vwb2xsX2NyZWF0ZSglZClcbiIsDQogCQkg ICAgIGN1cnJlbnQsIHNpemUpKTsNCiANCi0JLyogU2FuaXR5IGNoZWNrIG9u IHRoZSBzaXplIHBhcmFtZXRlciAqLw0KKwkvKg0KKwkgKiBTYW5pdHkgY2hl Y2sgb24gdGhlIHNpemUgcGFyYW1ldGVyLCBhbmQgY3JlYXRlIHRoZSBpbnRl cm5hbCBkYXRhDQorCSAqIHN0cnVjdHVyZSAoICJzdHJ1Y3QgZXZlbnRwb2xs IiApLg0KKwkgKi8NCiAJZXJyb3IgPSAtRUlOVkFMOw0KLQlpZiAoc2l6ZSA8 PSAwKQ0KKwlpZiAoc2l6ZSA8PSAwIHx8IChlcnJvciA9IGVwX2FsbG9jKCZl cCkpICE9IDApDQogCQlnb3RvIGVleGl0XzE7DQogDQogCS8qDQogCSAqIENy ZWF0ZXMgYWxsIHRoZSBpdGVtcyBuZWVkZWQgdG8gc2V0dXAgYW4gZXZlbnRw b2xsIGZpbGUuIFRoYXQgaXMsDQogCSAqIGEgZmlsZSBzdHJ1Y3R1cmUsIGFu ZCBpbm9kZSBhbmQgYSBmcmVlIGZpbGUgZGVzY3JpcHRvci4NCiAJICovDQot CWVycm9yID0gZXBfZ2V0ZmQoJmZkLCAmaW5vZGUsICZmaWxlKTsNCi0JaWYg KGVycm9yKQ0KLQkJZ290byBlZXhpdF8xOw0KLQ0KLQkvKiBTZXR1cCB0aGUg ZmlsZSBpbnRlcm5hbCBkYXRhIHN0cnVjdHVyZSAoICJzdHJ1Y3QgZXZlbnRw b2xsIiApICovDQotCWVycm9yID0gZXBfZmlsZV9pbml0KGZpbGUpOw0KKwll cnJvciA9IGVwX2dldGZkKCZmZCwgJmlub2RlLCAmZmlsZSwgZXApOw0KIAlp ZiAoZXJyb3IpDQogCQlnb3RvIGVleGl0XzI7DQogDQotDQogCUROUFJJTlRL KDMsIChLRVJOX0lORk8gIlslcF0gZXZlbnRwb2xsOiBzeXNfZXBvbGxfY3Jl YXRlKCVkKSA9ICVkXG4iLA0KIAkJICAgICBjdXJyZW50LCBzaXplLCBmZCkp Ow0KIA0KIAlyZXR1cm4gZmQ7DQogDQogZWV4aXRfMjoNCi0Jc3lzX2Nsb3Nl KGZkKTsNCisJZXBfZnJlZShlcCk7DQorCWtmcmVlKGVwKTsNCiBlZXhpdF8x Og0KIAlETlBSSU5USygzLCAoS0VSTl9JTkZPICJbJXBdIGV2ZW50cG9sbDog c3lzX2Vwb2xsX2NyZWF0ZSglZCkgPSAlZFxuIiwNCiAJCSAgICAgY3VycmVu dCwgc2l6ZSwgZXJyb3IpKTsNCkBAIC03MDYsNyArNzA2LDggQEANCiAvKg0K ICAqIENyZWF0ZXMgdGhlIGZpbGUgZGVzY3JpcHRvciB0byBiZSB1c2VkIGJ5 IHRoZSBlcG9sbCBpbnRlcmZhY2UuDQogICovDQotc3RhdGljIGludCBlcF9n ZXRmZChpbnQgKmVmZCwgc3RydWN0IGlub2RlICoqZWlub2RlLCBzdHJ1Y3Qg ZmlsZSAqKmVmaWxlKQ0KK3N0YXRpYyBpbnQgZXBfZ2V0ZmQoaW50ICplZmQs IHN0cnVjdCBpbm9kZSAqKmVpbm9kZSwgc3RydWN0IGZpbGUgKiplZmlsZSwN CisJCSAgICBzdHJ1Y3QgZXZlbnRwb2xsICplcCkNCiB7DQogCXN0cnVjdCBx c3RyIHRoaXM7DQogCWNoYXIgbmFtZVszMl07DQpAQCAtNzU2LDcgKzc1Nyw3 IEBADQogCWZpbGUtPmZfb3AgPSAmZXZlbnRwb2xsX2ZvcHM7DQogCWZpbGUt PmZfbW9kZSA9IEZNT0RFX1JFQUQ7DQogCWZpbGUtPmZfdmVyc2lvbiA9IDA7 DQotCWZpbGUtPnByaXZhdGVfZGF0YSA9IE5VTEw7DQorCWZpbGUtPnByaXZh dGVfZGF0YSA9IGVwOw0KIA0KIAkvKiBJbnN0YWxsIHRoZSBuZXcgc2V0dXAg ZmlsZSBpbnRvIHRoZSBhbGxvY2F0ZWQgZmQuICovDQogCWZkX2luc3RhbGwo ZmQsIGZpbGUpOw0KQEAgLTc3Nyw3ICs3NzgsNyBAQA0KIH0NCiANCiANCi1z dGF0aWMgaW50IGVwX2ZpbGVfaW5pdChzdHJ1Y3QgZmlsZSAqZmlsZSkNCitz dGF0aWMgaW50IGVwX2FsbG9jKHN0cnVjdCBldmVudHBvbGwgKipwZXApDQog ew0KIAlzdHJ1Y3QgZXZlbnRwb2xsICplcDsNCiANCkBAIC03OTIsOSArNzkz LDkgQEANCiAJSU5JVF9MSVNUX0hFQUQoJmVwLT5yZGxsaXN0KTsNCiAJZXAt PnJiciA9IFJCX1JPT1Q7DQogDQotCWZpbGUtPnByaXZhdGVfZGF0YSA9IGVw Ow0KKwkqcGVwID0gZXA7DQogDQotCUROUFJJTlRLKDMsIChLRVJOX0lORk8g IlslcF0gZXZlbnRwb2xsOiBlcF9maWxlX2luaXQoKSBlcD0lcFxuIiwNCisJ RE5QUklOVEsoMywgKEtFUk5fSU5GTyAiWyVwXSBldmVudHBvbGw6IGVwX2Fs bG9jKCkgZXA9JXBcbiIsDQogCQkgICAgIGN1cnJlbnQsIGVwKSk7DQogCXJl dHVybiAwOw0KIH0NCg== --8323328-1689195616-1126915013=:6125--
Re: [patch] Fix epoll delayed initialization bug ...
Author: Roland Dreier
Date: Fri, 16 Sep 2005 16:59
Date: Fri, 16 Sep 2005 16:59
8 lines
446 bytes
446 bytes
Davide> Al found a potential problem in epoll_create(), where the Davide> file->private_data member was set after fd_install(). This is Davide> obviously wrong since another thread might do a close() on Davide> that fd# before we set the file->private_data member. This Davide> goes over 2.6.13 and passes a few basic tests I've done here. Actually, I found the problem after Al pointed out a similar bug in my code ;) - R.
Re: [patch] Fix epoll delayed initialization bug ...
Author: Andrew Morton
Date: Fri, 16 Sep 2005 17:05
Date: Fri, 16 Sep 2005 17:05
19 lines
550 bytes
550 bytes
Davide Libenzi <davidel@xmailserver.org> wrote: > > > Sigh. Space-stuffing strikes again. Please resend as an attachment. > > > > The number of whitespace-buggered patches which are coming in is just > > getting out of control lately. > > > > Even `patch -l' tossed four rejects, so there may be something else wrong > > in this one. > > My side or your side? Not mine, pal ;) Although sylpheed could perhaps do a better job of decrypting some of the oddities which arrive. > Anyway, see if the one attached merges cleanly ... It does.
Re: [patch] Fix epoll delayed initialization bug ...
Author: Paul Jackson
Date: Fri, 16 Sep 2005 19:37
Date: Fri, 16 Sep 2005 19:37
26 lines
1105 bytes
1105 bytes
Andrew complained: > The number of whitespace-buggered patches which are coming in is just > getting out of control lately. If people didn't use email clients, but rather a patch sending script, it would work better. It is easier to send patch sets this way (you get to edit all the vital fields, such as Subject, To and Cc lists, in your favorite text editor), and the usual failures that mess up patch sending are avoided. The script I'm using is in good shape, and several others besides myself are successfully using it to send patches to lkml. See the embedded Usage string for documentation. http://www.speakeasy.org/~pj99/sgi/sendpatchset It handles sending one or several related patches, to a list of email addresses. You prepare a text directive file with the addresses, subjects and pathnames to the files containing the message contents. Then you send it all off with a single invocation of this 'sendpatchset' script. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
Re: [patch] Fix epoll delayed initialization bug ...
Author: Al Viro
Date: Sat, 17 Sep 2005 06:47
Date: Sat, 17 Sep 2005 06:47
9 lines
522 bytes
522 bytes
On Fri, Sep 16, 2005 at 04:59:02PM -0700, Roland Dreier wrote: > Davide> Al found a potential problem in epoll_create(), where the > Davide> file->private_data member was set after fd_install(). This is > Davide> obviously wrong since another thread might do a close() on > Davide> that fd# before we set the file->private_data member. This > Davide> goes over 2.6.13 and passes a few basic tests I've done here. > > Actually, I found the problem after Al pointed out a similar bug in my code ;) Yup.
Thread Navigation
This is a paginated view of messages in the thread with full content displayed inline.
Messages are displayed in chronological order, with the original post highlighted in green.
Use pagination controls to navigate through all messages in large threads.
Back to All Threads