public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Dave Jones <davej@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: BUG_ON(nd->inode->i_op->follow_link);
Date: Mon, 11 Mar 2013 00:02:07 +0000	[thread overview]
Message-ID: <20130311000207.GD21522@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFwObv7--izbae5YdXX3AhoHfV0YwbSLQoonTf-=e4PzqQ@mail.gmail.com>

On Thu, Mar 07, 2013 at 02:50:55PM -0800, Linus Torvalds wrote:

> and you get the oops, because when it follows that pseudo-symlink for
> fd3, it will hit that symlink we just looked up. And trigger the
> BUG_ON() you see.
> 
> Al, I think the BUG_ON() is simply bogus. Should we just remove it? Or
> is there some conceptual reason why we can't handle a symlink there?

BUG_ON() is bogus these days, but failing ->follow_link() is not.  The thing
is, what would you do with that sucker?  Suppose you have ->follow_link()
leaving you in a symlink.  What would you do with it?  Either you were
not going to follow it (in which case you wouldn't have called ->follow_link()
in the first place) *or* you need to know which directory it's in.  Which
we don't have in this case...

It really used to be an impossible case, until we'd introduced O_PATH.  And
IIRC, back when O_PATH had been developed, the plan was to fail attempts
to follow these suckers with -ELOOP; looks like we forgot to do that.  Mea
culpa...

And no, just removing BUG_ON() is not enough - callers of follow_link() will
do rather weird stuff (in nested symlink case it'll fail with ENOTDIR, in
path_lookupat() with LOOKUP_FOLLOW it'll give you a symlink, in case of
path_openat() with LOOKUP_FOLLOW fail with ELOOP).

See the patch I'd posted upthread.  Let's make it fail consistently...

  parent reply	other threads:[~2013-03-11  0:02 UTC|newest]

Thread overview: 99+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-07  2:16 BUG_ON(nd->inode != parent->d_inode); Dave Jones
2013-03-07 15:30 ` BUG_ON(nd->inode->i_op->follow_link); Dave Jones
2013-03-07 17:30   ` BUG_ON(nd->inode->i_op->follow_link); Linus Torvalds
2013-03-07 19:35     ` BUG_ON(nd->inode->i_op->follow_link); Dave Jones
2013-03-07 20:33       ` BUG_ON(nd->inode->i_op->follow_link); Linus Torvalds
2013-03-07 21:38         ` ipc/testmsg GPF Dave Jones
2013-03-07 21:45           ` Linus Torvalds
2013-03-07 21:49             ` David Miller
2013-03-07 21:51               ` Linus Torvalds
2013-03-07 22:03             ` Dave Jones
2013-03-07 22:36               ` pipe_release oops Dave Jones
2013-03-07 23:14                 ` fasync_remove_entry oops Dave Jones
2013-03-07 23:46                   ` Linus Torvalds
2013-03-07 23:54                     ` Dave Jones
2013-03-08  0:20                       ` Dave Jones
2013-03-08  0:21                 ` pipe_release oops Linus Torvalds
2013-03-08 14:53                   ` Dave Jones
2013-03-08 18:30                     ` Linus Torvalds
2013-03-08 18:26                       ` Jörn Engel
2013-03-10 23:33                         ` Al Viro
2013-03-12 19:09                           ` Jörn Engel
2013-03-10 22:10                       ` Al Viro
2013-03-11  0:35                         ` Al Viro
2013-03-11 15:10                           ` Linus Torvalds
2013-03-11 18:05                             ` Al Viro
2013-03-12 13:06                               ` Al Viro
2013-03-12 15:31                                 ` Linus Torvalds
2013-03-12 19:43                                   ` Al Viro
2013-03-12 19:56                                     ` Dave Jones
2013-03-12 20:09                                     ` Linus Torvalds
2013-03-12 20:51                                       ` Al Viro
2013-03-27 13:51                                       ` Yet another pipe related oops Dave Jones
2013-03-27 15:20                                         ` Al Viro
2013-03-27 16:33                                           ` Linus Torvalds
2013-03-27 16:53                                             ` Raymond Jennings
2013-03-27 17:45                                             ` Al Viro
2013-04-01 20:34                                               ` Al Viro
2013-04-01 21:00                                                 ` Greg Kroah-Hartman
2013-04-01 21:21                                                   ` Al Viro
2013-04-01 21:44                                                     ` Greg Kroah-Hartman
2013-04-01 23:27                                                       ` Al Viro
2013-04-02  0:22                                                         ` Al Viro
2013-04-02  1:55                                                           ` Greg Kroah-Hartman
2013-03-12  1:27                       ` pipe_release oops Dave Jones
2013-03-09  0:27           ` ipc/testmsg GPF Peter Hurley
2013-03-09  0:32             ` Dave Jones
2013-03-11 18:26             ` Dave Jones
2013-03-11 19:03               ` Peter Hurley
2013-03-12 22:02                 ` Andrew Morton
2013-03-12 22:33                   ` Dave Jones
2013-03-15 21:21                   ` Dave Jones
2013-03-25 16:37                 ` Dave Jones
2013-03-25 18:28                   ` Peter Hurley
2013-03-25 18:39                     ` Dave Jones
2013-03-07 22:18         ` BUG_ON(nd->inode->i_op->follow_link); Dave Jones
2013-03-07 22:50           ` BUG_ON(nd->inode->i_op->follow_link); Linus Torvalds
2013-03-07 23:03             ` BUG_ON(nd->inode->i_op->follow_link); Dave Jones
2013-03-07 23:55             ` BUG_ON(nd->inode->i_op->follow_link); Linus Torvalds
2013-03-11  0:02             ` Al Viro [this message]
2013-03-10 23:04   ` BUG_ON(nd->inode->i_op->follow_link); Al Viro
2013-03-12 18:31     ` BUG_ON(nd->inode->i_op->follow_link); Linus Torvalds
2013-03-08 15:04 ` BUG_ON(nd->inode != parent->d_inode); Dave Jones
2013-03-08 18:51   ` Linus Torvalds
2013-03-08 19:18     ` Dave Jones
2013-03-08 19:20       ` Dave Jones
2013-03-08 19:36         ` Dave Jones
2013-03-08 19:47           ` Linus Torvalds
2013-03-08 21:04             ` Dave Jones
2013-03-08 22:41               ` Linus Torvalds
2013-03-08 23:07                 ` Dave Jones
2013-03-08 23:14                   ` Dave Jones
2013-03-08 23:20                   ` Linus Torvalds
2013-03-08 23:28                     ` Linus Torvalds
2013-03-08 23:34                       ` Dave Jones
2013-03-08 23:47                       ` Dave Jones
2013-03-08 23:51                         ` Linus Torvalds
2013-03-08 23:30                     ` Dave Jones
2013-03-08 23:45                       ` Linus Torvalds
2013-03-08 23:55                         ` Dave Jones
2013-03-09  0:02                           ` Linus Torvalds
2013-03-09  0:19                             ` Dave Jones
2013-03-09  0:29                               ` Raymond Jennings
2013-03-09  0:36                               ` Dave Jones
2013-03-09  1:18                                 ` Linus Torvalds
2013-03-09  2:03                                   ` Dave Jones
2013-03-09  2:08                                     ` Linus Torvalds
2013-03-09  2:26                                       ` Dave Jones
2013-03-09  2:56                                         ` Dave Jones
2013-03-09  2:57                                           ` Dave Jones
     [not found]                                             ` <CA+55aFxyOYXnzDoWr7Utr1QLjjMUCON5EGH3FMvGBHxnxMJmQQ@mail.gmail.com>
2013-03-09  3:25                                               ` Dave Jones
2013-03-09  3:38                                                 ` Eric W. Biederman
2013-03-09  4:26                                                   ` Dave Jones
2013-03-09  8:28                                                     ` Eric W. Biederman
     [not found]                                                 ` <CA+55aFweyfew3VU79ZQV4otJcWiF0=xKXxDtADXcccNxGaqMwA@mail.gmail.com>
2013-03-09  3:50                                                   ` Dave Jones
2013-03-09  4:31                                                     ` Linus Torvalds
2013-03-09  4:39                                                       ` Dave Jones
2013-03-09  5:13                                                         ` Sasha Levin
2013-03-09  5:16                                                           ` Dave Jones
2013-03-09  3:27                                             ` 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=20130311000207.GD21522@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=davej@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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