Skip to contentSkip to conversation

staging: gdm72xx: fix address space parse warnings

PATCH Superseded Okash Khawaja 2 participants 7 messages Nov 29, 2015 – Dec 12, 2015 Staging

lore
Share & tools
lore

Okash Khawaja

author
#0 lore raw

PATCH 1/1 staging: gdm72xx: fix address space parse warnings

copy_to_user() and copy_from_user() functions expect the user space pointers to be marked with __user. Sparse throws following warnings.

For copy_to_user():

warning: incorrect type in argument 1 (different address spaces) expected void [noderef] <asn:1>*to
got void *buf

For copy_from_user():

warning: incorrect type in argument 2 (different address spaces) expected void const [noderef] <asn:1>*from
got void *buf

This patch casts the user space pointers to have __user by using __force.

Signed-off-by Okash Khawaja <[email protected]>
---
 drivers/staging/gdm72xx/gdm_wimax.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/staging/gdm72xx/gdm_wimax.c +8 -2
index d9ddced..16f8f35 100644
@@ -371,6 +371,7 @@ static void kdelete(void **buf)
 static int gdm_wimax_ioctl_get_data(struct data_s *dst, struct data_s *src)
 {
 	int size;
+	void __user *p;
 
 	size = dst->size < src->size ? dst->size : src->size;
 

@@ -378,7 +379,9 @@ static int gdm_wimax_ioctl_get_data(struct data_s *dst, struct data_s *src)
 	if (src->size) {
 		if (!dst->buf)
 			return -EINVAL;
-		if (copy_to_user(dst->buf, src->buf, size))
+
+		p = (__force void __user *)dst->buf;
+		if (copy_to_user(p, src->buf, size))
 			return -EFAULT;
 	}
 	return 0;

@@ -386,6 +389,8 @@ static int gdm_wimax_ioctl_get_data(struct data_s *dst, struct data_s *src)
 
 static int gdm_wimax_ioctl_set_data(struct data_s *dst, struct data_s *src)
 {
+	void __user *p;
+
 	if (!src->size) {
 		dst->size = 0;
 		return 0;

@@ -401,7 +406,8 @@ static int gdm_wimax_ioctl_set_data(struct data_s *dst, struct data_s *src)
 			return -ENOMEM;
 	}
 
-	if (copy_from_user(dst->buf, src->buf, src->size)) {
+	p = (__force void __user *)src->buf;
+	if (copy_from_user(dst->buf, p, src->size)) {
 		kdelete(&dst->buf);
 		return -EFAULT;
 	}
Signature · 1 line
2.5.2
#1 lore raw

Probably it's better to create a different struct:

struct udata_s {
	unsigned int size;
	void __user *buf;
};

regards,
dan carpenter

author
#2 lore raw

Thanks, that's a cleaner approach. Apologies for delayed reply as I'm in middle of a house move. I'll send an updated patch soon.

Okash

Quoted 13 lines · Dan Carpenter

Probably it's better to create a different struct:

struct udata_s {
   unsigned int size;
   void __user *buf;
};

regards,
dan carpenter

#3 lore raw
Quoted 2 lines · Okash Khawaja

Thanks, that's a cleaner approach. Apologies for delayed reply as I'm in middle of a house move. I'll send an updated patch soon.

Someone already fixed it. Thanks.

regards,
dan carpenter

author
#4 lore raw
Quoted 11 lines · Dan Carpenter
Quoted 3 lines · Okash Khawaja

Thanks, that's a cleaner approach. Apologies for delayed reply as I'm in middle of a house move. I'll send an updated patch soon.

Someone already fixed it. Thanks.

regards,
dan carpenter

It seems like it's still there: https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/staging/+/staging-next/drivers/staging/gdm72xx/gdm_wimax.c

Am I looking in the wrong place?

Thanks,
Okash

author
#5 lore raw

PATCH v2 1/1 staging: gdm72xx: fix address space warnings

In reply to Dan Carpenter

copy_to_user() and copy_from_user() functions expect the user space pointers to be marked with __user. Sparse throws following warnings.

For copy_to_user():

warning: incorrect type in argument 1 (different address spaces) expected void [noderef] <asn:1>*to
got void *buf

For copy_from_user():

warning: incorrect type in argument 2 (different address spaces) expected void const [noderef] <asn:1>*from
got void *buf

This creates a separate struct udata_s which is user space counterpart of struct data_s. This patch also alters function signatures where data transfer between user and kernel space occurs, in order to make the distinction explicit. Finally, it explicitly casts the argument passed to gdm_update_fsm() using __force to avoid the warning "cast removes address space of expression".

Signed-off-by Okash Khawaja <[email protected]>
---
 drivers/staging/gdm72xx/gdm_wimax.c | 7 ++++---
 drivers/staging/gdm72xx/wm_ioctl.h  | 7 ++++++-
 2 files changed, 10 insertions(+), 4 deletions(-)
drivers/staging/gdm72xx/gdm_wimax.c +4 -3
index b8eea21..c61ce92 100644
@@ -363,7 +363,7 @@ static void kdelete(void **buf)
 	}
 }
 
-static int gdm_wimax_ioctl_get_data(struct data_s *dst, struct data_s *src)
+static int gdm_wimax_ioctl_get_data(struct udata_s *dst, struct data_s *src)
 {
 	int size;
 

@@ -379,7 +379,7 @@ static int gdm_wimax_ioctl_get_data(struct data_s *dst, struct data_s *src)
 	return 0;
 }
 
-static int gdm_wimax_ioctl_set_data(struct data_s *dst, struct data_s *src)
+static int gdm_wimax_ioctl_set_data(struct data_s *dst, struct udata_s *src)
 {
 	if (!src->size) {
 		dst->size = 0;

@@ -478,7 +478,8 @@ static int gdm_wimax_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 				 * before gdm_wimax_ioctl_set_data is called.
 				 */
 				gdm_update_fsm(dev,
-					       req->data.buf);
+						(__force struct fsm_s *)
+						req->data.buf);
 			}
 			ret = gdm_wimax_ioctl_set_data(
 				&nic->sdk_data[req->data_id], &req->data);
drivers/staging/gdm72xx/wm_ioctl.h +6 -1
index ed8f649..7dc8f6f 100644
@@ -78,13 +78,18 @@ struct data_s {
 	void	*buf;
 };
 
+struct udata_s {
+	unsigned int size;
+	void __user *buf;
+};
+
 struct wm_req_s {
 	union {
 		char ifrn_name[IFNAMSIZ];
 	} ifr_ifrn;
 	unsigned short	cmd;
 	unsigned short	data_id;
-	struct data_s	data;
+	struct udata_s	data;
 
 /* NOTE: sizeof(struct wm_req_s) must be less than sizeof(struct ifreq). */
 };
Signature · 1 line
2.5.2