* [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type
@ 2021-01-28 14:49 Colin King
2021-01-28 15:05 ` Chuck Lever
0 siblings, 1 reply; 6+ messages in thread
From: Colin King @ 2021-01-28 14:49 UTC (permalink / raw)
To: J . Bruce Fields, Chuck Lever, linux-nfs; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The call to find_stateid_by_type is setting the return value in *stid
yet the NULL check of the return is checking stid instead of *stid.
Fix this by adding in the missing pointer * operator.
Addresses-Coverity: ("Dereference before null check")
Fixes: 6cdaa72d4dde ("nfsd: find_cpntf_state cleanup")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
fs/nfsd/nfs4state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f554e3480bb1..423fd6683f3a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5824,7 +5824,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
*stid = find_stateid_by_type(found, &cps->cp_p_stateid,
NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
- if (stid)
+ if (*stid)
status = nfs_ok;
else
status = nfserr_bad_stateid;
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type
2021-01-28 14:49 [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type Colin King
@ 2021-01-28 15:05 ` Chuck Lever
2021-01-28 15:17 ` Bruce Fields
2021-01-28 15:34 ` Dan Carpenter
0 siblings, 2 replies; 6+ messages in thread
From: Chuck Lever @ 2021-01-28 15:05 UTC (permalink / raw)
To: Colin King
Cc: Bruce Fields, Linux NFS Mailing List, kernel-janitors, linux-kernel
Hi Colin-
> On Jan 28, 2021, at 9:49 AM, Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> The call to find_stateid_by_type is setting the return value in *stid
> yet the NULL check of the return is checking stid instead of *stid.
> Fix this by adding in the missing pointer * operator.
>
> Addresses-Coverity: ("Dereference before null check")
> Fixes: 6cdaa72d4dde ("nfsd: find_cpntf_state cleanup")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Thanks for your patch. I've committed it to the for-next branch at
git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
in preparation for the v5.12 merge window, with the following changes:
- ^statid^stateid
- Fixes: tag removed, since no stable backport is necessary
The commit you are fixing has not been merged upstream yet.
> ---
> fs/nfsd/nfs4state.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index f554e3480bb1..423fd6683f3a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5824,7 +5824,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
>
> *stid = find_stateid_by_type(found, &cps->cp_p_stateid,
> NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
> - if (stid)
> + if (*stid)
> status = nfs_ok;
> else
> status = nfserr_bad_stateid;
> --
> 2.29.2
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type
2021-01-28 15:05 ` Chuck Lever
@ 2021-01-28 15:17 ` Bruce Fields
2021-01-28 15:34 ` Dan Carpenter
1 sibling, 0 replies; 6+ messages in thread
From: Bruce Fields @ 2021-01-28 15:17 UTC (permalink / raw)
To: Chuck Lever
Cc: Colin King, Linux NFS Mailing List, kernel-janitors, linux-kernel
On Thu, Jan 28, 2021 at 03:05:06PM +0000, Chuck Lever wrote:
> Hi Colin-
>
> > On Jan 28, 2021, at 9:49 AM, Colin King <colin.king@canonical.com> wrote:
> >
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > The call to find_stateid_by_type is setting the return value in *stid
> > yet the NULL check of the return is checking stid instead of *stid.
> > Fix this by adding in the missing pointer * operator.
> >
> > Addresses-Coverity: ("Dereference before null check")
> > Fixes: 6cdaa72d4dde ("nfsd: find_cpntf_state cleanup")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> Thanks for your patch. I've committed it to the for-next branch at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>
> in preparation for the v5.12 merge window, with the following changes:
>
> - ^statid^stateid
> - Fixes: tag removed, since no stable backport is necessary
Please keep the "Fixes:" tag! It's still very useful information. For
example, if someone needs to backport the original patch, this is a
reminder they'll want this one as well.
(Of course, if you fold this patch into the earlier one instead, that's
a different situation.)
--b.
> The commit you are fixing has not been merged upstream yet.
>
>
> > ---
> > fs/nfsd/nfs4state.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index f554e3480bb1..423fd6683f3a 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5824,7 +5824,7 @@ static __be32 find_cpntf_state(struct nfsd_net *nn, stateid_t *st,
> >
> > *stid = find_stateid_by_type(found, &cps->cp_p_stateid,
> > NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID);
> > - if (stid)
> > + if (*stid)
> > status = nfs_ok;
> > else
> > status = nfserr_bad_stateid;
> > --
> > 2.29.2
> >
>
> --
> Chuck Lever
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type
2021-01-28 15:05 ` Chuck Lever
2021-01-28 15:17 ` Bruce Fields
@ 2021-01-28 15:34 ` Dan Carpenter
2021-01-28 15:53 ` Chuck Lever
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2021-01-28 15:34 UTC (permalink / raw)
To: Chuck Lever
Cc: Colin King, Bruce Fields, Linux NFS Mailing List,
kernel-janitors, linux-kernel
On Thu, Jan 28, 2021 at 03:05:06PM +0000, Chuck Lever wrote:
> Hi Colin-
>
> > On Jan 28, 2021, at 9:49 AM, Colin King <colin.king@canonical.com> wrote:
> >
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > The call to find_stateid_by_type is setting the return value in *stid
> > yet the NULL check of the return is checking stid instead of *stid.
> > Fix this by adding in the missing pointer * operator.
> >
> > Addresses-Coverity: ("Dereference before null check")
> > Fixes: 6cdaa72d4dde ("nfsd: find_cpntf_state cleanup")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> Thanks for your patch. I've committed it to the for-next branch at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>
> in preparation for the v5.12 merge window, with the following changes:
>
> - ^statid^stateid
> - Fixes: tag removed, since no stable backport is necessary
>
> The commit you are fixing has not been merged upstream yet.
Fixes tags don't meant the patch has to be backported. Is your tree
rebased? In that case, the fixes tag probably doesn't make sense
because the tag can change. You might want to just consider folding
Colin's fix into the original commit.
Fixes tags are used for a lot of different things:
1) If there is a fixes tag, then you can tell it does *NOT* have to
be back ported because the original commit is not in the stable
tree. It saves time for the stable maintainers.
2) Metrics to figure out how quickly we are fixing bugs.
3) Sometimes the Fixes tag helps because we want to review the original
patch to see what the intent was.
All sorts of stuff. Etc.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type
2021-01-28 15:34 ` Dan Carpenter
@ 2021-01-28 15:53 ` Chuck Lever
2021-01-28 18:50 ` Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Chuck Lever @ 2021-01-28 15:53 UTC (permalink / raw)
To: Dan Carpenter
Cc: Colin King, Bruce Fields, Linux NFS Mailing List,
kernel-janitors, linux-kernel
Hi Dan-
> On Jan 28, 2021, at 10:34 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Thu, Jan 28, 2021 at 03:05:06PM +0000, Chuck Lever wrote:
>> Hi Colin-
>>
>>> On Jan 28, 2021, at 9:49 AM, Colin King <colin.king@canonical.com> wrote:
>>>
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> The call to find_stateid_by_type is setting the return value in *stid
>>> yet the NULL check of the return is checking stid instead of *stid.
>>> Fix this by adding in the missing pointer * operator.
>>>
>>> Addresses-Coverity: ("Dereference before null check")
>>> Fixes: 6cdaa72d4dde ("nfsd: find_cpntf_state cleanup")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>
>> Thanks for your patch. I've committed it to the for-next branch at
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>>
>> in preparation for the v5.12 merge window, with the following changes:
>>
>> - ^statid^stateid
>> - Fixes: tag removed, since no stable backport is necessary
>>
>> The commit you are fixing has not been merged upstream yet.
>
> Fixes tags don't meant the patch has to be backported. Is your tree
> rebased? In that case, the fixes tag probably doesn't make sense
> because the tag can change. You might want to just consider folding
> Colin's fix into the original commit.
Yes, this branch can be rebased on occasion. Since you and Bruce
suggest squashing the fix into the original patch, I will do that.
> Fixes tags are used for a lot of different things:
> 1) If there is a fixes tag, then you can tell it does *NOT* have to
> be back ported because the original commit is not in the stable
> tree. It saves time for the stable maintainers.
> 2) Metrics to figure out how quickly we are fixing bugs.
> 3) Sometimes the Fixes tag helps because we want to review the original
> patch to see what the intent was.
>
> All sorts of stuff. Etc.
Yep, I'm a fan of all that. I just want to avoid poking the stable
automation bear when it's unnecessary.
--
Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type
2021-01-28 15:53 ` Chuck Lever
@ 2021-01-28 18:50 ` Bruce Fields
0 siblings, 0 replies; 6+ messages in thread
From: Bruce Fields @ 2021-01-28 18:50 UTC (permalink / raw)
To: Chuck Lever
Cc: Dan Carpenter, Colin King, Linux NFS Mailing List,
kernel-janitors, linux-kernel
On Thu, Jan 28, 2021 at 03:53:36PM +0000, Chuck Lever wrote:
> > On Jan 28, 2021, at 10:34 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Fixes tags are used for a lot of different things:
> > 1) If there is a fixes tag, then you can tell it does *NOT* have to
> > be back ported because the original commit is not in the stable
> > tree. It saves time for the stable maintainers.
> > 2) Metrics to figure out how quickly we are fixing bugs.
> > 3) Sometimes the Fixes tag helps because we want to review the original
> > patch to see what the intent was.
> >
> > All sorts of stuff. Etc.
>
> Yep, I'm a fan of all that. I just want to avoid poking the stable
> automation bear when it's unnecessary.
I've routinely had patches with Fixes: lines referencing other queued-up
patches, and I didn't get any stable mail about it.
100% not something to worry about.
--b.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-28 18:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28 14:49 [PATCH][next] nfsd: fix check of statid returned from call to find_stateid_by_type Colin King
2021-01-28 15:05 ` Chuck Lever
2021-01-28 15:17 ` Bruce Fields
2021-01-28 15:34 ` Dan Carpenter
2021-01-28 15:53 ` Chuck Lever
2021-01-28 18:50 ` Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).