linux-kernel - Re: [patch 04/14] gfs2: dont call permission()

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]

Date:	Fri, 23 May 2008 11:48:28 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	hch@...radead.org
CC:	miklos@...redi.hu, linux-fsdevel@...r.kernel.org,
	hch@...radead.org, viro@...IV.linux.org.uk,
	linux-kernel@...r.kernel.org, swhiteho@...hat.com
Subject: Re: [patch 04/14] gfs2: dont call permission()

> Bad idea.  You're duplicating bits out of permission for no good
> reason.  I spent quite a bit of effort to make sure we don't have
> this duplicated logic around.

In this case you are wrong.  Look at the ugly conditional locking
gfs2_permission() does, which is probably due to the fact that it's
doing a recursion via calling permission() from inside already locked
parts in the filesystem.

That's _much_ worse than a duplicated IS_IMMUTABLE() call.  Which btw,
is a filesystem implementation detail: it needs to re-check the
immutability of the file after it has been locked.  I'm not even sure
it's strictly needed.  Steven?

Generally this sort of recursion through the VFS is ugly and
unnecessary, it's much better to provide helper for what the VFS is
doing if there's a lot of duplication.  But in this case there's
really no point in that at all.

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/