drivers: staging/lustre: conrpc.c - fix sparse error: incompatible types in comparison expression (different address spaces)

PATCH In review Guillermo O. Freschi 2 participants 8 messages May 5, 2017 – Jun 7, 2017 Staging

Share & tools

Guillermo O. Freschi

author

2017-05-05 13:51 UTC #0 lore raw

PATCH 1/1 drivers: staging/lustre: conrpc.c - fix sparse error: incompatible types in comparison expression (different address spaces)

Signed-off-by Guillermo O. Freschi <[email protected]>

---
 drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/lustre/lnet/selftest/conrpc.c +1 -1

index da36c55b86d3..f4578200994f 100644
@@ -487,7 +487,7 @@ lstcon_rpc_trans_interpreter(struct lstcon_rpc_trans *trans,
 				   sizeof(struct list_head)))
 			return -EFAULT;
 
-		if (tmp.next == head_up)
+		if ((struct list_head __user *)tmp.next == head_up)
 			return 0;
 
 		next = tmp.next;

Signature · 1 line

2.11.0

Tordek

author

2017-06-01 18:56 UTC #2 lore raw

PATCH 1/1 drivers: staging/lustre: conrpc.c - fix sparse error: incompatible types in comparison expression (different address spaces)

This is fairly minor but it reveals a few hidden warnings, could I get some feedback on it?

Thanks
--
Guillermo

Quoted 9 lines · Tordek

This patch makes a lot of new warnings pop up, because the error is blocking the rest of the file from being processed.
Quoted 5 lines · Guillermo O. Freschi
Signed-off-by Guillermo O. Freschi <[email protected]>
---
 drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 19 lines

Review
Quoted 1 line
(quoted diff) +0 -0
b/drivers/staging/lustre/lnet/selftest/conrpc.c
drivers/staging/lustre/lnet/selftest/conrpc.c lstcon_rpc_trans_interpreter(struct
Quoted 4 lines
index da36c55b86d3..f4578200994f 100644
drivers/staging/lustre/lnet/selftest/conrpc.c +0 -0
@@ -487,7 +487,7 @@ lstcon_rpc_trans_interpreter(struct
lstcon_rpc_trans *trans,
Quoted 11 lines
drivers/staging/lustre/lnet/selftest/conrpc.c +1 -2
                                   sizeof(struct list_head)))
                        return -EFAULT;
 
-               if (tmp.next == head_up)
+               if ((struct list_head __user *)tmp.next == head_up)
                        return 0;
 
                next = tmp.next;
--
2.11.0

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20170601/e759a8cd/attachment.htm>

Dilger, Andreas

2017-06-02 04:08 UTC #3 lore raw

Quoted 1 line · Tordek

This is fairly minor but it reveals a few hidden warnings, could I get some feedback on it?

This is a known problem, but can't immediately be fixed because it affects the interface with userspace tools. The correct solution is to not use struct list_head in the user interface at all, and we are looking into that.

In the meantime, feel free to look at the warnings that are now visible after this patch is applied (locally on your system).

Thanks, Andreas

Quoted 23 lines · Tordek

This patch makes a lot of new warnings pop up, because the error is blocking the rest of the file from being processed.
Quoted 17 lines · Guillermo O. Freschi
Signed-off-by Guillermo O. Freschi <[email protected]>
---
drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/lustre/lnet/selftest/conrpc.c +1 -1
index da36c55b86d3..f4578200994f 100644
@@ -487,7 +487,7 @@ lstcon_rpc_trans_interpreter(struct lstcon_rpc_trans *trans,
                                  sizeof(struct list_head)))
                       return -EFAULT;
 
-               if (tmp.next == head_up)
+               if ((struct list_head __user *)tmp.next == head_up)
                       return 0;
 

Quoted 8 lines

Quoted 4 lines
drivers/staging/lustre/lnet/selftest/conrpc.c +0 -1
               next = tmp.next;
--
2.11.0
_______________________________________________
lustre-devel mailing list
lustre-devel at lists.lustre.org
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation

Tordek

author

2017-06-02 05:15 UTC #4 lore raw

Quoted 9 lines · Andreas

Quoted 1 line · Tordek
This is fairly minor but it reveals a few hidden warnings, could I get some feedback on it?
This is a known problem, but can't immediately be fixed because it affects the interface with userspace tools. The correct solution is to not use struct list_head in the user interface at all, and we are looking into that.
In the meantime, feel free to look at the warnings that are now visible after this patch is applied (locally on your system).

Well, changing

struct lstcon_rpc_ent *ent;

struct lstcon_rpc_ent __user *ent;

makes most of the post-patch warnings disappear (and it... makes sense? we're calling copy_to_user(&ent... a lot, so I assume ent would be pointing to userspace memory).

But if this is meant to change, maybe it's a moot point to look at it at all.

Thanks!

Quoted 35 lines

Thanks, Andreas
Quoted 23 lines · Tordek
This patch makes a lot of new warnings pop up, because the error is blocking the rest of the file from being processed.
Quoted 17 lines · Guillermo O. Freschi
Signed-off-by Guillermo O. Freschi <[email protected]>
---
drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/lustre/lnet/selftest/conrpc.c +1 -1
index da36c55b86d3..f4578200994f 100644
@@ -487,7 +487,7 @@ lstcon_rpc_trans_interpreter(struct lstcon_rpc_trans *trans,
                                  sizeof(struct list_head)))
                       return -EFAULT;
 
-               if (tmp.next == head_up)
+               if ((struct list_head __user *)tmp.next == head_up)
                       return 0;
 
Quoted 8 lines
Quoted 4 lines
drivers/staging/lustre/lnet/selftest/conrpc.c +0 -1
               next = tmp.next;
--
2.11.0
_______________________________________________
lustre-devel mailing list
lustre-devel at lists.lustre.org
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Quoted 12 lines

Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel Corporation

--
Tordek

Guillermo O. Freschi

author

2017-06-07 09:15 UTC #5 lore raw

PATCH 1/2 drivers: staging/lustre: conrpc.c - fix sparse error: incompatible types in comparison expression (different address spaces)

In reply to Dilger, Andreas

This patch reveals hidden warnings after this line.

(This is a stepping-stone patch for the following one, it should be ignored as per Andreas Dilger.)

Signed-off-by Guillermo O. Freschi <[email protected]>

---
 drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/lustre/lnet/selftest/conrpc.c +1 -1

index da36c55b86d3..f4578200994f 100644
@@ -487,7 +487,7 @@ lstcon_rpc_trans_interpreter(struct lstcon_rpc_trans *trans,
 				   sizeof(struct list_head)))
 			return -EFAULT;
 
-		if (tmp.next == head_up)
+		if ((struct list_head __user *)tmp.next == head_up)
 			return 0;
 
 		next = tmp.next;

Signature · 1 line

2.11.0

Guillermo O. Freschi

author

2017-06-07 09:15 UTC #6 lore raw

PATCH 2/2 drivers: staging/lustre: conrpc.c - fix sparse warning: warning: incorrect type in argument * (different address spaces)

The calls to copy_to_user suggest that ent is pointing to userspace memory, but ent is missing the __user qualifier.

Signed-off-by Guillermo O. Freschi <[email protected]>

---
 drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/lustre/lnet/selftest/conrpc.c +1 -1

index f4578200994f..04a5d7e80151 100644
@@ -469,7 +469,7 @@ lstcon_rpc_trans_interpreter(struct lstcon_rpc_trans *trans,
 {
 	struct list_head tmp;
 	struct list_head __user *next;
-	struct lstcon_rpc_ent *ent;
+	struct lstcon_rpc_ent __user *ent;
 	struct srpc_generic_reply *rep;
 	struct lstcon_rpc *crpc;
 	struct srpc_msg *msg;

Signature · 1 line

2.11.0

Tordek

author

2017-06-01 21:42 UTC #7 lore raw

In reply to Tordek

This is fairly minor but it reveals a few hidden warnings, could I get some feedback on it?

Thanks
--
Guillermo

Quoted 25 lines · Tordek

This patch makes a lot of new warnings pop up, because the error is blocking the rest of the file from being processed.

Quoted 21 lines · Guillermo O. Freschi

Signed-off-by Guillermo O. Freschi <[email protected]>

---
 drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/lustre/lnet/selftest/conrpc.c +1 -2

index da36c55b86d3..f4578200994f 100644
@@ -487,7 +487,7 @@ lstcon_rpc_trans_interpreter(struct lstcon_rpc_trans *trans,
                                   sizeof(struct list_head)))
                        return -EFAULT;
 
-               if (tmp.next == head_up)
+               if ((struct list_head __user *)tmp.next == head_up)
                        return 0;
 
                next = tmp.next;
--

2.11.0