🚀 go-pugleaf

RetroBBS NetNews Server

Inspired by RockSolid Light RIP Retro Guy

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 ...
#307566
Author: Davide Libenzi
Date: Fri, 16 Sep 2005 16:35
117 lines
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 ...
#307569
Author: Andrew Morton
Date: Fri, 16 Sep 2005 16:50
22 lines
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 ...
#307573
Author: Davide Libenzi
Date: Fri, 16 Sep 2005 16:56
113 lines
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 ...
#307574
Author: Roland Dreier
Date: Fri, 16 Sep 2005 16:59
8 lines
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 ...
#307577
Author: Andrew Morton
Date: Fri, 16 Sep 2005 17:05
19 lines
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 ...
#307605
Author: Paul Jackson
Date: Fri, 16 Sep 2005 19:37
26 lines
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 ...
#307624
Author: Al Viro
Date: Sat, 17 Sep 2005 06:47
9 lines
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