lustre/lnet/lnet/lib-move.c: memcpy with (struct iovec*)->iov_base, which is __user

lustre/lnet/lnet/lib-move.c: memcpy with (struct iovec*)->iov_base, which is __user

[Andrey Utkin]

Dear maintainers of LustreFS and other experienced kernel developers!

I am working on fixing some sparse warnings as a task of Eudyptula Challenge.
There's a thing that look suspiciously to me (or I just don't
understand it). This looks same both in upstream kernel code in
drivers/staging/ and in https://github.com/Xyratex/lustre-stable.git
(it doesn't fetch from http://git.whamcloud.com/fs/lustre-release.git,
but I guess it is similar with Xyratex repo).

In lnet/lnet/lib-move.c, in lnet_copy_iov2iov(), at line 209, there is

                memcpy((char *)diov->iov_base + doffset,
                        (char *)siov->iov_base + soffset, this_nob);

diov and siov are struct iovec *, and iov_base in it is void __user *.
Are diov and siov really used semantically correctly here, holding
userspace addresses in iov_base?
Is it semantically correct to use memcpy to copy bytes from one
userspace address to another?

How to treat the comment "/* NB diov, siov are READ-ONLY */" at line
177 in the same procedure? Is it adequate?

This above-mentioned memcpy operation causes sparse warnings "warning:
cast removes address space of expression" for both first and second
line. When i change it to

                memcpy((void __user *)((char __user *)diov->iov_base + doffset),
                        (void __user *)((char __user *)siov->iov_base
+ soffset), this_nob);

It gives other sparse warnings:
drivers/staging/lustre/lnet/lnet/lib-move.c:208:25: warning: incorrect
type in argument 1 (different address spaces)
drivers/staging/lustre/lnet/lnet/lib-move.c:208:25:    expected void *to
drivers/staging/lustre/lnet/lnet/lib-move.c:208:25:    got void
[noderef] <asn:1>*<noident>
drivers/staging/lustre/lnet/lnet/lib-move.c:209:26: warning: incorrect
type in argument 2 (different address spaces)
drivers/staging/lustre/lnet/lnet/lib-move.c:209:26:    expected void const *from
drivers/staging/lustre/lnet/lnet/lib-move.c:209:26:    got void
[noderef] <asn:1>*<noident>

What is supposed way to avoid warnings in this case?

Thanks.

-- 
Andrey Utkin

Re: lustre/lnet/lnet/lib-move.c: memcpy with (struct iovec*)->iov_base, which is __user

[Dan Carpenter]

Didn't Al change these to kvec instead of iovec?  You have to look at
the callers to figure out if it's actually a user space pointer or a
kernel pointer.

regards,
dan carpenter

Re: lustre/lnet/lnet/lib-move.c: memcpy with (struct iovec*)->iov_base, which is __user

[Andrey Utkin]

2015-01-07 10:36 GMT+02:00 Dan Carpenter <dan.carpenter@oracle.com>:
> Didn't Al change these to kvec instead of iovec?  You have to look at
> the callers to figure out if it's actually a user space pointer or a
> kernel pointer.

I am looking at current linux-next checkout.

-- 
Andrey Utkin

Re: lustre/lnet/lnet/lib-move.c: memcpy with (struct iovec*)->iov_base, which is __user

[Dilger, Andreas]

On 2015/01/07, 1:36 AM, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:
> Didn't Al change these to kvec instead of iovec?  You have to look at
> the callers to figure out if it's actually a user space pointer or a
> kernel pointer.

A patch was sent by Al on Dec 2 to replace iovec with kvec, in a thread
titled "[PATCH] staging:lustre:lnet: Incorrect type in assignment".  Greg
replied on Dec 10 in another thread (also fixing this same warning) titled
"[PATCH] staging: lustre: Fix the warning messages about casting without
__user macro" that Al's patch was in the staging tree for 3.19-rc1.

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division

Re: lustre/lnet/lnet/lib-move.c: memcpy with (struct iovec*)->iov_base, which is __user

[Andrey Utkin]

2015-01-07 13:32 GMT+02:00 Dilger, Andreas <andreas.dilger@intel.com>:
> On 2015/01/07, 1:36 AM, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:
>> Didn't Al change these to kvec instead of iovec?  You have to look at
>> the callers to figure out if it's actually a user space pointer or a
>> kernel pointer.
>
> A patch was sent by Al on Dec 2 to replace iovec with kvec, in a thread
> titled "[PATCH] staging:lustre:lnet: Incorrect type in assignment".  Greg
> replied on Dec 10 in another thread (also fixing this same warning) titled
> "[PATCH] staging: lustre: Fix the warning messages about casting without
> __user macro" that Al's patch was in the staging tree for 3.19-rc1.

In current linux-next, I see none of the mentioned patches. Also I
don't see them mentioned in GKH's merge commit "Merge tag
'staging-3.19-rc1'..."
Is it ok?

-- 
Andrey Utkin

Re: lustre/lnet/lnet/lib-move.c: memcpy with (struct iovec*)->iov_base, which is __user

[Greg KH]

On Wed, Jan 07, 2015 at 02:28:12PM +0200, Andrey Utkin wrote:
> 2015-01-07 13:32 GMT+02:00 Dilger, Andreas <andreas.dilger@intel.com>:
> > On 2015/01/07, 1:36 AM, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:
> >> Didn't Al change these to kvec instead of iovec?  You have to look at
> >> the callers to figure out if it's actually a user space pointer or a
> >> kernel pointer.
> >
> > A patch was sent by Al on Dec 2 to replace iovec with kvec, in a thread
> > titled "[PATCH] staging:lustre:lnet: Incorrect type in assignment".  Greg
> > replied on Dec 10 in another thread (also fixing this same warning) titled
> > "[PATCH] staging: lustre: Fix the warning messages about casting without
> > __user macro" that Al's patch was in the staging tree for 3.19-rc1.
> 
> In current linux-next, I see none of the mentioned patches. Also I
> don't see them mentioned in GKH's merge commit "Merge tag
> 'staging-3.19-rc1'..."
> Is it ok?

Al's patch is now merged.