All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>, Sergey Dyasly <dserrg@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/4] proc: avoid ->f_pos overflows in proc_task_readdir() paths
Date: Tue, 4 Jun 2013 19:14:35 +0200	[thread overview]
Message-ID: <20130604171435.GA20416@redhat.com> (raw)
In-Reply-To: <877giarg81.fsf@xmission.com>

On 06/03, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@redhat.com> writes:
>
> > 1. proc_task_readdir() truncates f_pos to long, this can lead
> >    to wrong result on 32bit.
> >
> This won't compile on some 32bit architectures like x86-32.
>
> switch(unsigned long long) requires helpers that the kernel does not
> included.  Or at least the kernel has not included because such code
> is a problem.  In fact that is the reason Linus put the case to
> unsigned long in there.

Hmm, ee568b25, suprise... I am wondering if the kernel still supports
the compilers which needs __cmpdi2 in this case...

But this doesn't matter. The patch should not blindly revert ee568b25,
thanks!

> There is another bug in here as well that we may return really crazy
> things in the case of seek simultaneous with readdir.

i_mutex? both vfs_readdir() and default_llseek() take it... Anyway, this
is another issue.

> I think we really need to put f_pos
> in a local variable.

Heh. I swear, this is what I did initially. But I was afraid you will
blame this change as "overcomplicated" ;)

OK, I'll redo/resend this one later.

Oleg.


  reply	other threads:[~2013-06-04 17:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-03 19:06 [PATCH v2 0/4] proc: first_tid() fix/cleanup Oleg Nesterov
2013-06-03 19:06 ` [PATCH v2 1/4] proc: first_tid: fix the potential use-after-free Oleg Nesterov
2013-06-03 19:07 ` [PATCH v2 2/4] proc: change first_tid() to use while_each_thread() Oleg Nesterov
2013-06-03 19:07 ` [PATCH v2 3/4] proc: simplify proc_task_readdir/first_tid paths Oleg Nesterov
2013-06-03 22:06   ` Eric W. Biederman
2013-06-03 19:07 ` [PATCH v2 4/4] proc: avoid ->f_pos overflows in proc_task_readdir() paths Oleg Nesterov
2013-06-03 22:18   ` Eric W. Biederman
2013-06-04 17:14     ` Oleg Nesterov [this message]
2013-06-04 17:39       ` Al Viro
2013-06-04 19:57         ` Oleg Nesterov
2013-06-04 21:06           ` Al Viro
2013-06-04  0:58   ` Al Viro
2013-06-04 17:35     ` Oleg Nesterov
2013-06-04 17:32 ` [PATCH v2 0/4] proc: first_tid() fix/cleanup Oleg Nesterov

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=20130604171435.GA20416@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dserrg@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@suse.cz \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.