Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1611509 > unrolled thread
| Started by | Marcos Paulo de Souza <marcos.souza.org@gmail.com> |
|---|---|
| First post | 2017-03-29 04:20 +0200 |
| Last post | 2017-03-29 13:00 +0200 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] staging: lusten: conrpc.c: fix different address space sparse warning Marcos Paulo de Souza <marcos.souza.org@gmail.com> - 2017-03-29 04:20 +0200
Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning Greg KH <gregkh@linuxfoundation.org> - 2017-03-29 09:40 +0200
Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning Marcos Paulo de Souza <marcos.souza.org@gmail.com> - 2017-03-29 12:30 +0200
Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning Greg KH <gregkh@linuxfoundation.org> - 2017-03-29 12:40 +0200
Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning Marcos Paulo de Souza <marcos.souza.org@gmail.com> - 2017-03-29 13:00 +0200
| From | Marcos Paulo de Souza <marcos.souza.org@gmail.com> |
|---|---|
| Date | 2017-03-29 04:20 +0200 |
| Subject | [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning |
| Message-ID | <tqbnk-bP-9@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Greg KH <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2017-03-29 09:40 +0200 |
| Subject | Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning |
| Message-ID | <tqgn1-3Eh-37@gated-at.bofh.it> |
| In reply to | #1611509 |
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
[toc] | [prev] | [next] | [standalone]
| From | Marcos Paulo de Souza <marcos.souza.org@gmail.com> |
|---|---|
| Date | 2017-03-29 12:30 +0200 |
| Subject | Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning |
| Message-ID | <tqj1w-5wl-15@gated-at.bofh.it> |
| In reply to | #1611658 |
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
[toc] | [prev] | [next] | [standalone]
| From | Greg KH <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2017-03-29 12:40 +0200 |
| Subject | Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning |
| Message-ID | <tqjbd-5zA-41@gated-at.bofh.it> |
| In reply to | #1611791 |
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
[toc] | [prev] | [next] | [standalone]
| From | Marcos Paulo de Souza <marcos.souza.org@gmail.com> |
|---|---|
| Date | 2017-03-29 13:00 +0200 |
| Subject | Re: [PATCH] staging: lusten: conrpc.c: fix different address space sparse warning |
| Message-ID | <tqjux-5Ii-5@gated-at.bofh.it> |
| In reply to | #1611802 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web