* [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning @ 2017-03-29 2:14 Marcos Paulo de Souza 2017-03-29 7:31 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Marcos Paulo de Souza @ 2017-03-29 2:14 UTC (permalink / raw) To: gregkh Cc: Marcos Paulo de Souza, Oleg Drokin, Andreas Dilger, James Simmons, Doug Oucharek, Al Viro, lustre-devel, devel, linux-kernel head_up parameter is marked with __user attribute, tmp is filled by a copy_from_user from next, that is also marked as __user, so tmp.next needs to be "casted" as __user to make sparse happy. Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- this is mt first patch addressing an issue of sparse, so let me know if I misunderstood the error message drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c index c6a683b..fb7ad74 100644 --- a/drivers/staging/lustre/lnet/selftest/conrpc.c +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c @@ -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.9.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [lustre-devel] [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning 2017-03-29 2:14 [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning Marcos Paulo de Souza @ 2017-03-29 7:31 ` Greg KH 0 siblings, 0 replies; 7+ messages in thread From: Greg KH @ 2017-03-29 7:31 UTC (permalink / raw) To: Marcos Paulo de Souza Cc: devel, Doug Oucharek, linux-kernel, Oleg Drokin, Al Viro, Andreas Dilger, lustre-devel On Tue, Mar 28, 2017 at 11:14:06PM -0300, Marcos Paulo de Souza wrote: > head_up parameter is marked with __user attribute, tmp is filled > by a copy_from_user from next, that is also marked as __user, so > tmp.next needs to be "casted" as __user to make sparse happy. But is it the correct change? You also have a typo in your subject :( > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> > --- > > this is mt first patch addressing an issue of sparse, so let me know > if I misunderstood the error message > > drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c > index c6a683b..fb7ad74 100644 > --- a/drivers/staging/lustre/lnet/selftest/conrpc.c > +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c > @@ -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) Aer you sure this is correct? __user changes for lustre is not trivial... How did you test this? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning @ 2017-03-29 7:31 ` Greg KH 0 siblings, 0 replies; 7+ messages in thread From: Greg KH @ 2017-03-29 7:31 UTC (permalink / raw) To: Marcos Paulo de Souza Cc: devel, Doug Oucharek, linux-kernel, Oleg Drokin, Al Viro, Andreas Dilger, lustre-devel On Tue, Mar 28, 2017 at 11:14:06PM -0300, Marcos Paulo de Souza wrote: > head_up parameter is marked with __user attribute, tmp is filled > by a copy_from_user from next, that is also marked as __user, so > tmp.next needs to be "casted" as __user to make sparse happy. But is it the correct change? You also have a typo in your subject :( > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> > --- > > this is mt first patch addressing an issue of sparse, so let me know > if I misunderstood the error message > > drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c > index c6a683b..fb7ad74 100644 > --- a/drivers/staging/lustre/lnet/selftest/conrpc.c > +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c > @@ -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) Aer you sure this is correct? __user changes for lustre is not trivial... How did you test this? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning 2017-03-29 7:31 ` Greg KH (?) @ 2017-03-23 19:09 ` Marcos Paulo de Souza 2017-03-29 10:34 ` Greg KH -1 siblings, 1 reply; 7+ messages in thread From: Marcos Paulo de Souza @ 2017-03-23 19:09 UTC (permalink / raw) To: Greg KH Cc: devel, Doug Oucharek, linux-kernel, Oleg Drokin, Al Viro, Andreas Dilger, lustre-devel On Wed, Mar 29, 2017 at 09:31:14AM +0200, Greg KH wrote: > On Tue, Mar 28, 2017 at 11:14:06PM -0300, Marcos Paulo de Souza wrote: > > head_up parameter is marked with __user attribute, tmp is filled > > by a copy_from_user from next, that is also marked as __user, so > > tmp.next needs to be "casted" as __user to make sparse happy. > > But is it the correct change? I don't know, it's my first sparse patch, so I tried to fix this warning. > > You also have a typo in your subject :( Sorry, didn't noticed yesterday :( > > > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> > > --- > > > > this is mt first patch addressing an issue of sparse, so let me know > > if I misunderstood the error message > > > > drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c > > index c6a683b..fb7ad74 100644 > > --- a/drivers/staging/lustre/lnet/selftest/conrpc.c > > +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c > > @@ -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) > > Aer you sure this is correct? __user changes for lustre is not > trivial... > > How did you test this? I didn't tested, it just removed the warning. Is this a false positive? > > thanks, > > greg k-h -- Thanks, Marcos ^ permalink raw reply [flat|nested] 7+ messages in thread
* [lustre-devel] [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning 2017-03-23 19:09 ` Marcos Paulo de Souza @ 2017-03-29 10:34 ` Greg KH 0 siblings, 0 replies; 7+ messages in thread From: Greg KH @ 2017-03-29 10:34 UTC (permalink / raw) To: Marcos Paulo de Souza Cc: devel, Doug Oucharek, linux-kernel, Oleg Drokin, Al Viro, Andreas Dilger, lustre-devel On Thu, Mar 23, 2017 at 04:09:03PM -0300, Marcos Paulo de Souza wrote: > On Wed, Mar 29, 2017 at 09:31:14AM +0200, Greg KH wrote: > > On Tue, Mar 28, 2017 at 11:14:06PM -0300, Marcos Paulo de Souza wrote: > > > head_up parameter is marked with __user attribute, tmp is filled > > > by a copy_from_user from next, that is also marked as __user, so > > > tmp.next needs to be "casted" as __user to make sparse happy. > > > > But is it the correct change? > > I don't know, it's my first sparse patch, so I tried to fix this > warning. > > > > > You also have a typo in your subject :( > > Sorry, didn't noticed yesterday :( > > > > > > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> > > > --- > > > > > > this is mt first patch addressing an issue of sparse, so let me know > > > if I misunderstood the error message > > > > > > drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c > > > index c6a683b..fb7ad74 100644 > > > --- a/drivers/staging/lustre/lnet/selftest/conrpc.c > > > +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c > > > @@ -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) > > > > Aer you sure this is correct? __user changes for lustre is not > > trivial... > > > > How did you test this? > > I didn't tested, it just removed the warning. Is this a false positive? I don't know, it's up to you to prove to me that you know this change is correct. You have to justify your changes, and "because checkpatch.pl complained" isn't a valid justification for something like this :) thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning @ 2017-03-29 10:34 ` Greg KH 0 siblings, 0 replies; 7+ messages in thread From: Greg KH @ 2017-03-29 10:34 UTC (permalink / raw) To: Marcos Paulo de Souza Cc: devel, Doug Oucharek, linux-kernel, Oleg Drokin, Al Viro, Andreas Dilger, lustre-devel On Thu, Mar 23, 2017 at 04:09:03PM -0300, Marcos Paulo de Souza wrote: > On Wed, Mar 29, 2017 at 09:31:14AM +0200, Greg KH wrote: > > On Tue, Mar 28, 2017 at 11:14:06PM -0300, Marcos Paulo de Souza wrote: > > > head_up parameter is marked with __user attribute, tmp is filled > > > by a copy_from_user from next, that is also marked as __user, so > > > tmp.next needs to be "casted" as __user to make sparse happy. > > > > But is it the correct change? > > I don't know, it's my first sparse patch, so I tried to fix this > warning. > > > > > You also have a typo in your subject :( > > Sorry, didn't noticed yesterday :( > > > > > > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> > > > --- > > > > > > this is mt first patch addressing an issue of sparse, so let me know > > > if I misunderstood the error message > > > > > > drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c > > > index c6a683b..fb7ad74 100644 > > > --- a/drivers/staging/lustre/lnet/selftest/conrpc.c > > > +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c > > > @@ -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) > > > > Aer you sure this is correct? __user changes for lustre is not > > trivial... > > > > How did you test this? > > I didn't tested, it just removed the warning. Is this a false positive? I don't know, it's up to you to prove to me that you know this change is correct. You have to justify your changes, and "because checkpatch.pl complained" isn't a valid justification for something like this :) thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning 2017-03-29 10:34 ` Greg KH (?) @ 2017-03-23 19:30 ` Marcos Paulo de Souza -1 siblings, 0 replies; 7+ messages in thread From: Marcos Paulo de Souza @ 2017-03-23 19:30 UTC (permalink / raw) To: Greg KH Cc: devel, Doug Oucharek, linux-kernel, Oleg Drokin, Al Viro, Andreas Dilger, lustre-devel On Wed, Mar 29, 2017 at 12:34:05PM +0200, Greg KH wrote: > On Thu, Mar 23, 2017 at 04:09:03PM -0300, Marcos Paulo de Souza wrote: > > On Wed, Mar 29, 2017 at 09:31:14AM +0200, Greg KH wrote: > > > On Tue, Mar 28, 2017 at 11:14:06PM -0300, Marcos Paulo de Souza wrote: > > > > head_up parameter is marked with __user attribute, tmp is filled > > > > by a copy_from_user from next, that is also marked as __user, so > > > > tmp.next needs to be "casted" as __user to make sparse happy. > > > > > > But is it the correct change? > > > > I don't know, it's my first sparse patch, so I tried to fix this > > warning. > > > > > > > > You also have a typo in your subject :( > > > > Sorry, didn't noticed yesterday :( > > > > > > > > > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> > > > > --- > > > > > > > > this is mt first patch addressing an issue of sparse, so let me know > > > > if I misunderstood the error message > > > > > > > > drivers/staging/lustre/lnet/selftest/conrpc.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/lustre/lnet/selftest/conrpc.c b/drivers/staging/lustre/lnet/selftest/conrpc.c > > > > index c6a683b..fb7ad74 100644 > > > > --- a/drivers/staging/lustre/lnet/selftest/conrpc.c > > > > +++ b/drivers/staging/lustre/lnet/selftest/conrpc.c > > > > @@ -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) > > > > > > Aer you sure this is correct? __user changes for lustre is not > > > trivial... > > > > > > How did you test this? > > > > I didn't tested, it just removed the warning. Is this a false positive? > > I don't know, it's up to you to prove to me that you know this change is > correct. You have to justify your changes, and "because checkpatch.pl > complained" isn't a valid justification for something like this :) Fair enough, I'll take in another sparse report to work on. Thanks! > > thanks, > > greg k-h -- Thanks, Marcos ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-29 10:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-29 2:14 [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning Marcos Paulo de Souza 2017-03-29 7:31 ` [lustre-devel] " Greg KH 2017-03-29 7:31 ` Greg KH 2017-03-23 19:09 ` Marcos Paulo de Souza 2017-03-29 10:34 ` [lustre-devel] " Greg KH 2017-03-29 10:34 ` Greg KH 2017-03-23 19:30 ` Marcos Paulo de Souza
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.