public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Zhang Le <r0bertz@gentoo.org>
Cc: linux-kernel@vger.kernel.org, torvalds@osdl.org, alan@redhat.com,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: [PATCH] filp->f_pos not correctly updated in proc_task_readdir
Date: Tue, 17 Mar 2009 11:12:48 -0700	[thread overview]
Message-ID: <m17i2okogv.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <1237185871-3343-1-git-send-email-r0bertz@gentoo.org> (Zhang Le's message of "Mon\, 16 Mar 2009 14\:44\:31 +0800")

Zhang Le <r0bertz@gentoo.org> writes:

> filp->f_pos only get updated at the end of the function. Thus d_off of those
> dirents who are in the middle will be 0, and this will cause a problem in
> glibc's readdir implementation, specifically endless loop. Because when overflow
> occurs, f_pos will be set to next dirent to read, however it will be 0, unless
> the next one is the last one. So it will start over again and again.
>
> There is a sample program in man 2 gendents. This is the output of the program
> running on a multithread program's task dir before this patch is applied:
>
> $ ./a.out /proc/3807/task
> --------------- nread=128 ---------------
> i-node#  file type  d_reclen  d_off   d_name
>   506442  directory    16          1  .
>   506441  directory    16          0  ..
>   506443  directory    16          0  3807
>   506444  directory    16          0  3809
>   506445  directory    16          0  3812
>   506446  directory    16          0  3861
>   506447  directory    16          0  3862
>   506448  directory    16          8  3863
>
> This is the output after this patch is applied
>
> $ ./a.out /proc/3807/task
> --------------- nread=128 ---------------
> i-node#  file type  d_reclen  d_off   d_name
>   506442  directory    16          1  .
>   506441  directory    16          2  ..
>   506443  directory    16          3  3807
>   506444  directory    16          4  3809
>   506445  directory    16          5  3812
>   506446  directory    16          6  3861
>   506447  directory    16          7  3862
>   506448  directory    16          8  3863

I'm trying to understand what the problem that glibc
runs into.  Is this the glibc seekdir madness?

Under which conditions have you seen this problem?

The reason I ask is if this is triggered by seekdir and people really
care then the current proc_task_readdir for the /proc/<pid>/task/
directories is not quite sufficient.  As threads come and go the d_off
associated with a specific thread will change.

Which means we probably should be returning:

$ ./a.out /proc/3807/task
--------------- nread=128 ---------------
i-node#  file type  d_reclen  d_off   d_name
  506442  directory    16          1  .
  506441  directory    16          2  ..
  506443  directory    16          3809  3807
  506444  directory    16          3811  3809
  506445  directory    16          3814  3812
  506446  directory    16          3863  3861
  506447  directory    16          3864  3862
  506448  directory    16          3865  3863

Which is slightly better unfortunately it doesn't give us the
guarantee that the d_off will be continually increasing.

Eric

  parent reply	other threads:[~2009-03-17 18:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-16  6:44 [PATCH] filp->f_pos not correctly updated in proc_task_readdir Zhang Le
2009-03-16  8:34 ` Al Viro
2009-03-16 16:27   ` Zhang Le
2009-03-17 10:37   ` Alexey Dobriyan
2009-03-17 12:53     ` Zhang Le
2009-03-17 13:48       ` Alexey Dobriyan
2009-03-17 18:11       ` Al Viro
2009-03-17 18:12 ` Eric W. Biederman [this message]
2009-03-18  7:27   ` Zhang Le
2009-03-18 10:36     ` Eric W. Biederman
2009-03-18 13:39       ` [PATCH] proc: Fix proc_tid_readdir so it handles the weird cases Eric W. Biederman
2009-03-18 13:57         ` Louis Rilling
2009-03-19  3:40           ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m17i2okogv.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=adobriyan@gmail.com \
    --cc=alan@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=r0bertz@gentoo.org \
    --cc=torvalds@osdl.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox