[PATCH] fat sparse fixes

[PATCH] fat sparse fixes

[Andries.Brouwer]

diff -u --recursive --new-file -X /linux/dontdiff a/fs/fat/dir.c b/fs/fat/dir.c
--- a/fs/fat/dir.c	Mon Sep 29 01:05:41 2003
+++ b/fs/fat/dir.c	Mon Sep 29 01:11:39 2003
@@ -630,7 +630,7 @@
 		    put_user(slen, &d1->d_reclen))
 			goto efault;
 	} else {
-		if (put_user(0, d2->d_name)			||
+		if (put_user(0, d2->d_name+0)			||
 		    put_user(0, &d2->d_reclen)			||
 		    copy_to_user(d1->d_name, name, len)		||
 		    put_user(0, d1->d_name+len)			||
@@ -663,7 +663,7 @@
 		return -EINVAL;
 	}
 
-	d1 = (struct dirent *)arg;
+	d1 = (struct dirent __user *)arg;
 	if (!access_ok(VERIFY_WRITE, d1, sizeof(struct dirent[2])))
 		return -EFAULT;
 	/*

Re: [PATCH] fat sparse fixes

[Linus Torvalds]

On Mon, 29 Sep 2003 Andries.Brouwer@cwi.nl wrote:
>
> --- a/fs/fat/dir.c	Mon Sep 29 01:05:41 2003
> +++ b/fs/fat/dir.c	Mon Sep 29 01:11:39 2003
> @@ -630,7 +630,7 @@
>  		    put_user(slen, &d1->d_reclen))
>  			goto efault;
>  	} else {
> -		if (put_user(0, d2->d_name)			||
> +		if (put_user(0, d2->d_name+0)			||
>  		    put_user(0, &d2->d_reclen)			||
>  		    copy_to_user(d1->d_name, name, len)		||
>  		    put_user(0, d1->d_name+len)			||

The above seems to just work around a sparse bug. Please don't - I'd 
rather have regular code and try to fix the sparse problem.

Hmm.. I wonder why sparse doesn't get the address space right on arrays. 
It should see that "d2" is a user pointer , so d2->d_name is one too.

It gets it right if you add the "+0", or if you add a "&" in front. So 
it looks like the sparse array->pointer degeneration misses something.

		Linus

Re: [PATCH] fat sparse fixes

[Christopher Li]

On Mon, Sep 29, 2003 at 09:50:54AM -0700, Linus Torvalds wrote:
> 
> On Mon, 29 Sep 2003 Andries.Brouwer@cwi.nl wrote:
> >
> > --- a/fs/fat/dir.c	Mon Sep 29 01:05:41 2003
> > +++ b/fs/fat/dir.c	Mon Sep 29 01:11:39 2003
> > @@ -630,7 +630,7 @@
> >  		    put_user(slen, &d1->d_reclen))
> >  			goto efault;
> >  	} else {
> > -		if (put_user(0, d2->d_name)			||
> > +		if (put_user(0, d2->d_name+0)			||
> >  		    put_user(0, &d2->d_reclen)			||
> >  		    copy_to_user(d1->d_name, name, len)		||
> >  		    put_user(0, d1->d_name+len)			||
> 
> The above seems to just work around a sparse bug. Please don't - I'd 
> rather have regular code and try to fix the sparse problem.
> 
> Hmm.. I wonder why sparse doesn't get the address space right on arrays. 
> It should see that "d2" is a user pointer , so d2->d_name is one too.

. The problem is in "*d2->d_name", the address space get
lost at evaluate_dereference of "*". It is a monster macro right
there. The simple version is:

struct dentry {
        char d_name[256];
};

int foo (void) {
        struct dentry __attribute__((noderef, address_space(1))) *d2;
        __typeof__(*d2->d_name) *__pu_addr = d2->d_name;
                   ^^^^^^^^^^^
}


 
> It gets it right if you add the "+0", or if you add a "&" in front. So 
> it looks like the sparse array->pointer degeneration misses something.

Besids address sapce, it seems that the source and target base type
is pointer of char_ctype instead of pointer of void_ctype. I get lost
there.

Regards.

Chris

Re: [PATCH] fat sparse fixes

[Linus Torvalds]

On Tue, 30 Sep 2003, Christopher Li wrote:
> 
> The problem is in "*d2->d_name", the address space get
> lost at evaluate_dereference of "*"

Yes, but taking the address of it should still undo all the things.  
"evaluate_addressof()" does the right (fairly complex) magic, but
apparently the "degenerate()" function does not.

Ho humm.. degenerate() really should be 100% the same as 
"evaluate_addressof()", but I'm sure I had some reason for doing them 
separately.

Probably a very bad reason, brought on by terminal mental illness. But a
reason none-the-less..

		Linus