2014-11-25 20:44:31

by Zahari Doychev

[permalink] [raw]
Subject: [PATCH] staging: lustre: fix pointer declarations

This patch fixes pointer declarations from void * to void __user * in order
to remove some sparse warnings.

lib-lnet.h:798:48: warning: incorrect type in initializer (different address spaces)
lib-lnet.h:798:48: expected void [noderef] <asn:1>*iov_base
lib-lnet.h:798:48: got void *dest
lib-lnet.h:787:47: warning: incorrect type in initializer (different address spaces)
lib-lnet.h:787:47: expected void [noderef] <asn:1>*iov_base
lib-lnet.h:787:47: got void *dest
lib-lnet.h:819:48: warning: incorrect type in initializer (different address spaces)
lib-lnet.h:819:48: expected void [noderef] <asn:1>*iov_base
lib-lnet.h:819:48: got void *src
lib-lnet.h:808:47: warning: incorrect type in initializer (different address spaces)
lib-lnet.h:808:47: expected void [noderef] <asn:1>*iov_base
lib-lnet.h:808:47: got void *src

Signed-off-by: Zahari Doychev <[email protected]>
---
drivers/staging/lustre/include/linux/lnet/lib-lnet.h | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
index 7e89b3b..e60ce56 100644
--- a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
+++ b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
@@ -780,22 +780,22 @@ void lnet_copy_kiov2kiov(unsigned int ndkiov, lnet_kiov_t *dkiov,
unsigned int soffset, unsigned int nob);

static inline void
-lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset,
+lnet_copy_iov2flat(int dlen, void __user *dest, unsigned int doffset,
unsigned int nsiov, struct iovec *siov, unsigned int soffset,
unsigned int nob)
{
- struct iovec diov = {/*.iov_base = */ dest, /*.iov_len = */ dlen};
+ struct iovec diov = {.iov_base = dest, .iov_len = dlen};

lnet_copy_iov2iov(1, &diov, doffset,
nsiov, siov, soffset, nob);
}

static inline void
-lnet_copy_kiov2flat(int dlen, void *dest, unsigned int doffset,
+lnet_copy_kiov2flat(int dlen, void __user *dest, unsigned int doffset,
unsigned int nsiov, lnet_kiov_t *skiov,
unsigned int soffset, unsigned int nob)
{
- struct iovec diov = {/* .iov_base = */ dest, /* .iov_len = */ dlen};
+ struct iovec diov = {.iov_base = dest, .iov_len = dlen};

lnet_copy_kiov2iov(1, &diov, doffset,
nsiov, skiov, soffset, nob);
@@ -803,9 +803,10 @@ lnet_copy_kiov2flat(int dlen, void *dest, unsigned int doffset,

static inline void
lnet_copy_flat2iov(unsigned int ndiov, struct iovec *diov, unsigned int doffset,
- int slen, void *src, unsigned int soffset, unsigned int nob)
+ int slen, void __user *src, unsigned int soffset,
+ unsigned int nob)
{
- struct iovec siov = {/*.iov_base = */ src, /*.iov_len = */slen};
+ struct iovec siov = {.iov_base = src, .iov_len = slen};

lnet_copy_iov2iov(ndiov, diov, doffset,
1, &siov, soffset, nob);
@@ -813,10 +814,10 @@ lnet_copy_flat2iov(unsigned int ndiov, struct iovec *diov, unsigned int doffset,

static inline void
lnet_copy_flat2kiov(unsigned int ndiov, lnet_kiov_t *dkiov,
- unsigned int doffset, int slen, void *src,
+ unsigned int doffset, int slen, void __user *src,
unsigned int soffset, unsigned int nob)
{
- struct iovec siov = {/* .iov_base = */ src, /* .iov_len = */ slen};
+ struct iovec siov = {.iov_base = src, .iov_len = slen};

lnet_copy_iov2kiov(ndiov, dkiov, doffset,
1, &siov, soffset, nob);
--
2.1.0


2014-11-26 02:05:47

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] staging: lustre: fix pointer declarations

On Tue, 2014-11-25 at 21:44 +0100, Zahari Doychev wrote:
> This patch fixes pointer declarations from void * to void __user * in order
> to remove some sparse warnings.

This patch does more than that.

Please make sure to describe all of the changes
in a patch in the commit message.

> diff --git a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
[]
> @@ -780,22 +780,22 @@ void lnet_copy_kiov2kiov(unsigned int ndkiov, lnet_kiov_t *dkiov,
> unsigned int soffset, unsigned int nob);
>
> static inline void
> -lnet_copy_iov2flat(int dlen, void *dest, unsigned int doffset,
> +lnet_copy_iov2flat(int dlen, void __user *dest, unsigned int doffset,
> unsigned int nsiov, struct iovec *siov, unsigned int soffset,
> unsigned int nob)
> {
> - struct iovec diov = {/*.iov_base = */ dest, /*.iov_len = */ dlen};
> + struct iovec diov = {.iov_base = dest, .iov_len = dlen};

Now using named initializers too.

2014-11-26 12:45:23

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] staging: lustre: fix pointer declarations

On Tue, Nov 25, 2014 at 09:44:21PM +0100, Zahari Doychev wrote:
> This patch fixes pointer declarations from void * to void __user * in order
> to remove some sparse warnings.

_Are_ those userland addresses, though? Quick grep shows that in the
only caller of lnet_copy_iov2flat() we have something called ibmsg passed
as the second argument *AND* *RIGHT* *BEFORE* *THAT* *CALL* *WE* *HAVE*
ibmsg = tx->tx_msg;
ibmsg->ibm_u.immediate.ibim_hdr = *hdr;
Go ahead, explain how does that manage to work if ibmsg is a userland pointer.
Either you have discovered an exploitable hole (direct store to userland
address), or it's not a userland pointer, after all.

Al, sick and tired of the "remove some warnings" as the sole rationale for
patches, without even an attempt to figure out what those warnings are
about. Magic box makes noises, magic box must be appeased...