[PATCH] filldir write data missing size

[PATCH] filldir write data missing size

[Yoshinori Sato]

"loff_t" is long long.
But "d_off" is unsigned long.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

diff --git a/fs/readdir.c b/fs/readdir.c
index 4e026e5..01e7152
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -159,7 +159,7 @@ static int filldir(void * __buf, const char * name, int namlen, loff_t offset,
 		return -EOVERFLOW;
 	dirent = buf->previous;
 	if (dirent) {
-		if (__put_user(offset, &dirent->d_off))
+		if (__put_user((unsigned long)offset, &dirent->d_off))
 			goto efault;
 	}
 	dirent = buf->current_dir;

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [PATCH] filldir write data missing size

[OGAWA Hirofumi]

Yoshinori Sato <ysato@users.sourceforge.jp> writes:

> "loff_t" is long long.
> But "d_off" is unsigned long.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
>
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 4e026e5..01e7152
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -159,7 +159,7 @@ static int filldir(void * __buf, const char * name, int namlen, loff_t offset,
>  		return -EOVERFLOW;
>  	dirent = buf->previous;
>  	if (dirent) {
> -		if (__put_user(offset, &dirent->d_off))
> +		if (__put_user((unsigned long)offset, &dirent->d_off))

Um.. __put_user() should be already doing it automatically..

>  			goto efault;
>  	}
>  	dirent = buf->current_dir;
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] filldir write data missing size

[Yoshinori Sato]

At Sun, 20 Jul 2008 02:24:28 +0900,
OGAWA Hirofumi wrote:
> 
> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
> 
> > "loff_t" is long long.
> > But "d_off" is unsigned long.
> >
> > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> >
> > diff --git a/fs/readdir.c b/fs/readdir.c
> > index 4e026e5..01e7152
> > --- a/fs/readdir.c
> > +++ b/fs/readdir.c
> > @@ -159,7 +159,7 @@ static int filldir(void * __buf, const char * name, int namlen, loff_t offset,
> >  		return -EOVERFLOW;
> >  	dirent = buf->previous;
> >  	if (dirent) {
> > -		if (__put_user(offset, &dirent->d_off))
> > +		if (__put_user((unsigned long)offset, &dirent->d_off))
> 
> Um.. __put_user() should be already doing it automatically..

I'm mistake.

I checked object, and found bad code.
This problem fix __put_user.
Thanks.
 
> >  			goto efault;
> >  	}
> >  	dirent = buf->current_dir;
> -- 
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [PATCH] filldir write data missing size

[OGAWA Hirofumi]

Yoshinori Sato <ysato@users.sourceforge.jp> writes:

>> > -		if (__put_user(offset, &dirent->d_off))
>> > +		if (__put_user((unsigned long)offset, &dirent->d_off))
>> 
>> Um.. __put_user() should be already doing it automatically..
>
> I'm mistake.
>
> I checked object, and found bad code.
> This problem fix __put_user.

Um.. Could you explain the detail of problem? Is this a workaround or
something for the bug of some compiler?  If so, shouldn't we fix
__put_user() instead of caller?

E.g. the following or something fixes it? (btw, this is for x86)

#define __put_user(x, ptr) ({						\
 	/* Since some compiler generates wrong code, we need __v.?? */	\
        __typeof__(*(ptr)) __v = (x);					\
	__put_user_nocheck(__v, (ptr), sizeof(*(ptr)));			\
})
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] filldir write data missing size

[Yoshinori Sato]

At Sun, 20 Jul 2008 05:11:29 +0900,
OGAWA Hirofumi wrote:
> 
> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
> 
> >> > -		if (__put_user(offset, &dirent->d_off))
> >> > +		if (__put_user((unsigned long)offset, &dirent->d_off))
> >> 
> >> Um.. __put_user() should be already doing it automatically..
> >
> > I'm mistake.
> >
> > I checked object, and found bad code.
> > This problem fix __put_user.
> 
> Um.. Could you explain the detail of problem? Is this a workaround or
> something for the bug of some compiler?  If so, shouldn't we fix
> __put_user() instead of caller?
> 
> E.g. the following or something fixes it? (btw, this is for x86)
> 
> #define __put_user(x, ptr) ({						\
>  	/* Since some compiler generates wrong code, we need __v.?? */	\
>         __typeof__(*(ptr)) __v = (x);					\
> 	__put_user_nocheck(__v, (ptr), sizeof(*(ptr)));			\
> })
> -- 
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

This problem is sh-linux-gcc v4.1.2 and target sh2(a)-bigendian.

"__put_user(s64, u32_ptr)" compiled.

Correct code.
*u32_ptr = s64 & 0xffffffff;

Bad code.
*u32_ptr = s64 >> 32;

I'm add cast put_user 4byte case.

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [PATCH] filldir write data missing size

[OGAWA Hirofumi]

Yoshinori Sato <ysato@users.sourceforge.jp> writes:

> This problem is sh-linux-gcc v4.1.2 and target sh2(a)-bigendian.
>
> "__put_user(s64, u32_ptr)" compiled.
>
> Correct code.
> *u32_ptr = s64 & 0xffffffff;
>
> Bad code.
> *u32_ptr = s64 >> 32;
>
> I'm add cast put_user 4byte case.

I see. How about the following patch? I guess the problems of this type
should be fixed.

diff -puN include/asm-sh/uaccess_32.h~sh-fix include/asm-sh/uaccess_32.h
--- linux-2.6/include/asm-sh/uaccess_32.h~sh-fix	2008-07-22 06:20:15.000000000 +0900
+++ linux-2.6-hirofumi/include/asm-sh/uaccess_32.h	2008-07-22 06:25:26.000000000 +0900
@@ -211,8 +211,9 @@ do {							\
 ({								\
 	long __pu_err;						\
 	__typeof__(*(ptr)) __user *__pu_addr = (ptr);		\
+	__typeof__(*(ptr)) __pu_val = x;			\
 	__chk_user_ptr(ptr);					\
-	__put_user_size((x), __pu_addr, (size), __pu_err);	\
+	__put_user_size(__pu_val, __pu_addr, (size), __pu_err);	\
 	__pu_err;						\
 })
 
@@ -220,8 +221,9 @@ do {							\
 ({								\
 	long __pu_err = -EFAULT;				\
 	__typeof__(*(ptr)) __user *__pu_addr = (ptr);		\
+	__typeof__(*(ptr)) __pu_val = x;			\
 	if (likely(access_ok(VERIFY_WRITE, __pu_addr, size)))	\
-		__put_user_size((x), __pu_addr, (size),		\
+		__put_user_size(__pu_val, __pu_addr, (size),	\
 				__pu_err);			\
 	__pu_err;						\
 })
_
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] filldir write data missing size

[Yoshinori Sato]

At Tue, 22 Jul 2008 06:31:12 +0900,
OGAWA Hirofumi wrote:
> 
> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
> 
> > This problem is sh-linux-gcc v4.1.2 and target sh2(a)-bigendian.
> >
> > "__put_user(s64, u32_ptr)" compiled.
> >
> > Correct code.
> > *u32_ptr = s64 & 0xffffffff;
> >
> > Bad code.
> > *u32_ptr = s64 >> 32;
> >
> > I'm add cast put_user 4byte case.
> 
> I see. How about the following patch? I guess the problems of this type
> should be fixed.
> 
> diff -puN include/asm-sh/uaccess_32.h~sh-fix include/asm-sh/uaccess_32.h
> --- linux-2.6/include/asm-sh/uaccess_32.h~sh-fix	2008-07-22 06:20:15.000000000 +0900
> +++ linux-2.6-hirofumi/include/asm-sh/uaccess_32.h	2008-07-22 06:25:26.000000000 +0900
> @@ -211,8 +211,9 @@ do {							\
>  ({								\
>  	long __pu_err;						\
>  	__typeof__(*(ptr)) __user *__pu_addr = (ptr);		\
> +	__typeof__(*(ptr)) __pu_val = x;			\
>  	__chk_user_ptr(ptr);					\
> -	__put_user_size((x), __pu_addr, (size), __pu_err);	\
> +	__put_user_size(__pu_val, __pu_addr, (size), __pu_err);	\
>  	__pu_err;						\
>  })
>  
> @@ -220,8 +221,9 @@ do {							\
>  ({								\
>  	long __pu_err = -EFAULT;				\
>  	__typeof__(*(ptr)) __user *__pu_addr = (ptr);		\
> +	__typeof__(*(ptr)) __pu_val = x;			\
>  	if (likely(access_ok(VERIFY_WRITE, __pu_addr, size)))	\
> -		__put_user_size((x), __pu_addr, (size),		\
> +		__put_user_size(__pu_val, __pu_addr, (size),	\
>  				__pu_err);			\
>  	__pu_err;						\
>  })
> _
> -- 
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Sorry too late reply.

This fix is good. 
But I send other workaround fix.

Thanks.

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

Re: [PATCH] filldir write data missing size

[Paul Mundt]

On Tue, Jul 22, 2008 at 06:31:12AM +0900, OGAWA Hirofumi wrote:
> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
> 
> > This problem is sh-linux-gcc v4.1.2 and target sh2(a)-bigendian.
> >
> > "__put_user(s64, u32_ptr)" compiled.
> >
> > Correct code.
> > *u32_ptr = s64 & 0xffffffff;
> >
> > Bad code.
> > *u32_ptr = s64 >> 32;
> >
> > I'm add cast put_user 4byte case.
> 
> I see. How about the following patch? I guess the problems of this type
> should be fixed.
> 
I merged a different workaround as no one bothered to CC me on this
thread. Your fix looks more correct though, please provide a
Signed-off-by for it and I'll take this over the previous workaround.

Re: [PATCH] filldir write data missing size

[OGAWA Hirofumi]

Paul Mundt <lethal@linux-sh.org> writes:

> On Tue, Jul 22, 2008 at 06:31:12AM +0900, OGAWA Hirofumi wrote:
>> Yoshinori Sato <ysato@users.sourceforge.jp> writes:
>> 
>> > This problem is sh-linux-gcc v4.1.2 and target sh2(a)-bigendian.
>> >
>> > "__put_user(s64, u32_ptr)" compiled.
>> >
>> > Correct code.
>> > *u32_ptr = s64 & 0xffffffff;
>> >
>> > Bad code.
>> > *u32_ptr = s64 >> 32;
>> >
>> > I'm add cast put_user 4byte case.
>> 
>> I see. How about the following patch? I guess the problems of this type
>> should be fixed.
>> 
> I merged a different workaround as no one bothered to CC me on this
> thread. Your fix looks more correct though, please provide a
> Signed-off-by for it and I'll take this over the previous workaround.

Of course. But I didn't test it at all, so please test it.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>