Is Rational rational?

Is Rational rational?

[Thomas Molina]

Does the enclosed patch to fs/namei.c make sense?

IBM/Rational Software is advocating use of the following patch to deal 
with a situation where users of Clearcase are having difficulties deleting 
sockets, devices, and named pipes when using the mvfs file system.  The 
problem reportedly arises from populating a shadow filesystem with the 
contents of the system root and the handling of sockets, devices and named 
pipes.  The full text of the problem report response can be accessed by 
going to:

http://www.ibm.com/software/rational/support/

and entering 1154790 in the search box.


--- /usr/src/linux/fs/namei.c Tue Oct 30 10:03:52 2001
+++ /usr/src/linux-patched/namei.c Fri Jun 20 05:12:16 2003
@@ -907,7 +907,7 @@
static inline int may_delete(struct inode *dir,struct dentry *victim, int isdir)
{
int error;
- if (!victim->d_inode || victim->d_parent->d_inode != dir)
+ if (!victim->d_inode)
return -ENOENT;
error = permission(dir,MAY_WRITE | MAY_EXEC);
if (error)

I'm not really competent to evaluate this proposesd patch, but it 
certainly makes me nervous.  Their comment on this also bothers me: 
"Rational Software believes that the check that is removed by this patch
is one that should never fail for any properly operating filesystem. "

Re: Is Rational rational?

[Linus Torvalds]

On Mon, 15 Dec 2003, Thomas Molina wrote:
>
> Does the enclosed patch to fs/namei.c make sense?

Hmm.. It may. Al is the final arbiter in this area, but from my fuzzy
memory, the whole (and only) point of that parent check is because we
didn't have proper locking for the whole path between rename/unlink. As
far as I can tell, we do the locking right, and the test is nonsensical
these days.

So what used to happen is that we could move the entry during the lookup,
and now the "rm" would succeed even though the "mv" to another directory
had _also_ succeeded (which is illogical).

Al? Does your recollection match mine?

> I'm not really competent to evaluate this proposesd patch, but it
> certainly makes me nervous.  Their comment on this also bothers me:
> "Rational Software believes that the check that is removed by this patch
> is one that should never fail for any properly operating filesystem. "

I think they are right. But I may have overlooked something really subtle.

		Linus

> --- /usr/src/linux/fs/namei.c Tue Oct 30 10:03:52 2001
> +++ /usr/src/linux-patched/namei.c Fri Jun 20 05:12:16 2003
> @@ -907,7 +907,7 @@
> static inline int may_delete(struct inode *dir,struct dentry *victim, int isdir)
> {
>  	int error;
> -	if (!victim->d_inode || victim->d_parent->d_inode != dir)
> +	if (!victim->d_inode)
>  		return -ENOENT;
>  	error = permission(dir,MAY_WRITE | MAY_EXEC);
>  	if (error)

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

Re: Is Rational rational?

[viro]

On Mon, Dec 15, 2003 at 09:59:58PM -0500, Thomas Molina wrote:
> static inline int may_delete(struct inode *dir,struct dentry *victim, int isdir)
> {
> int error;
> - if (!victim->d_inode || victim->d_parent->d_inode != dir)
> + if (!victim->d_inode)
> return -ENOENT;

+ BUG_ON(victim->d_parent->d_inode != dir);

HAND.
 
> I'm not really competent to evaluate this proposesd patch, but it 
> certainly makes me nervous.  Their comment on this also bothers me: 
> "Rational Software believes that the check that is removed by this patch
> is one that should never fail for any properly operating filesystem. "

Precisely.  It does fail for their code, though.  Obvious conclusions are left
as an exercise to readers.