linux-kernel - Re: nfs: infinite loop in fcntl(F

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <E1JkOff-0000pS-A7@pomaz-ex.szeredi.hu>
Date:	Fri, 11 Apr 2008 21:12:23 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	bfields@...ldses.org
CC:	trond.myklebust@....uio.no, miklos@...redi.hu,
	eshel@...aden.ibm.com, neilb@...e.de, akpm@...ux-foundation.org,
	linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: nfs: infinite loop in fcntl(F_SETLKW)

> > OK. So the correct fix here should really be applied to fcntl_setlk().
> > There is absolutely no reason why we should be looping at all if the
> > filesystem has a ->lock() method.
> > 
> > In fact, this looping behaviour was introduced recently in commit
> > 7723ec9777d9832849b76475b1a21a2872a40d20.
> 
> Apologies, that was indeed a behavioral change introduced in a commit
> that claimed just to be shuffling code around.

Yeah, that patch looks totally wrong.  It's not generally a good idea
to do a loop where the exit condition depends on something you don't
control.  And error values from filesystem methods are typically like
that.  For example with fuse, the error code could come from an
unprivileged userspace process.

I didn't realize this aspect of the bug previously, because I
concentrated on the lockd inconsistency.

Btw, why hasn't this work been posted on -fsdevel prior to merging
into mainline?

> 
> > Marc, Bruce, why was this
> > done, and how are filesystems now supposed to behave?
> > 
> 
> The assumption must have been that -EAGAIN could only mean that we
> needed to keep blocking, and hence was a nonsensical return from a
> filesystem lock method that waited itself for the lock to become
> available--such a method would return 0, -EINTR (or some more exotic
> error), or continue waiting.

EAGAIN for a blocking lock is nonsensical, so my original patch could
still make sense.  But that's no longer a regression, and not all that
important.

Miklos
--
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/