linux-kernel - Re: [pnfs] [GIT BISECT] first bad commit: 1f36f774 Switch !O_CREAT case to use of do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4BAB9D53.4000909@panasas.com>
Date:	Thu, 25 Mar 2010 19:28:51 +0200
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Al Viro <viro@...IV.linux.org.uk>
CC:	Benny Halevy <bhalevy@...asas.com>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	"J. Bruce Fields" <bfields@...i.umich.edu>,
	pNFS Mailing List <pnfs@...ux-nfs.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Doug Nazar <nazard.lkml@...il.com>
Subject: Re: [pnfs] [GIT BISECT] first bad commit: 1f36f774 Switch !O_CREAT
 case to use of do_last()

On 03/25/2010 05:25 PM, Al Viro wrote:
> On Thu, Mar 25, 2010 at 04:27:13PM +0200, Boaz Harrosh wrote:
>  
>> And about the NFS thing, let misbehaving applications byte there own bullets.
>> But a good app like git, does put an O_DIRECTORY on the directory open, and
>> therefore will receive consistent results, right?
> 
> What the hell do you mean, "misbehaving"?  O_DIRECTORY means one thing:
> "refuse to open if it turns out to be a non-directory".  If open() succeed
> with it and fails without, it's a definite bug.  Application has every
> right to call the damn thing without that flag.
> 

Ha, OK, sorry I thought exactly the opposite. I assumed it meant
"don't allow a directory to be opened, unless set". As an application
hint of saying I'm going to do file things to this handle, don't want
any dirs.

So yes definitely it looks like an NFS bug masked by the fact that everyone
is using opendir() form libc for this.

> It *can* be used as a hint by fs code, and it's a useful hint, so yes,
> we need to restore LOOKUP_DIRECTORY (until we get rid of the entire
> d_revalidate/lookup+open mess and pass struct file + open flags directly
> to fs method for the last step of pathname resolution; basically, most
> of the do_last() guts would become that method, killing the intents
> crap for good; new method would simply check for O_DIRECTORY presence
> instead of looking at LOOKUP_DIRECTORY).
> 
> However, NFS behaviour in cases when O_DIRECTORY is absent is a separate
> (and quite real) bug.  If nfs_atomic_lookup() sees -EISDIR from attempt
> to open on server, it should either issue plain lookup or try to use the
> information it got in response to open attempt (if there's enough of it
> there).  Passing -EISDIR to ->lookup() caller is simply wrong.
> 
> I'm going to send a fix for O_DIRECTORY case (restoring the behaviour
> we had in 2.6.33) today, but NFS side of things also needs to be dealt
> with.

Thank you Al for fixing this, I hope some capable NFS person will take
that issue to heart.

Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/