* re: IB/core: Use GID table in AH creation and dmac resolution
@ 2015-11-03 13:11 Dan Carpenter
2015-11-03 13:44 ` Matan Barak
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2015-11-03 13:11 UTC (permalink / raw)
To: matanb-VPRAkNaXOzVWk0Htik3J/w; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Matan Barak,
This is a semi-automatic email about new static checker warnings.
The patch dbf727de7440: "IB/core: Use GID table in AH creation and
dmac resolution" from Oct 15, 2015, leads to the following Smatch
complaint:
drivers/infiniband/hw/ocrdma/ocrdma_ah.c:157 ocrdma_create_ah()
error: we previously assumed 'sgid_attr.ndev' could be null (see line 146)
drivers/infiniband/hw/ocrdma/ocrdma_ah.c
145 }
146 if (sgid_attr.ndev) {
^^^^^^^^^^^^^^
Patch introduces a NULL check.
147 if (is_vlan_dev(sgid_attr.ndev))
148 vlan_tag = vlan_dev_vlan_id(sgid_attr.ndev);
149 dev_put(sgid_attr.ndev);
150 }
151
152 if ((pd->uctx) &&
153 (!rdma_is_multicast_addr((struct in6_addr *)attr->grh.dgid.raw)) &&
154 (!rdma_link_local_addr((struct in6_addr *)attr->grh.dgid.raw))) {
155 status = rdma_addr_find_dmac_by_grh(&sgid, &attr->grh.dgid,
156 attr->dmac, &vlan_tag,
157 sgid_attr.ndev->ifindex);
^^^^^^^^^^^^^^^^
Patch introduces this new dereference. The warning might be a false
positive if "pd->uctx" or rdma_is_multicast_addr() imply it's non-NULL
but I don't know this code well enough to say for sure. Hence this
email. :)
158 if (status) {
159 pr_err("%s(): Failed to resolve dmac from gid."
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: IB/core: Use GID table in AH creation and dmac resolution
2015-11-03 13:11 IB/core: Use GID table in AH creation and dmac resolution Dan Carpenter
@ 2015-11-03 13:44 ` Matan Barak
[not found] ` <CAOBf=msvVXCw129URdiWTLogGOpAjzboCB=h0haTh4wJZG5XXw@mail.gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Matan Barak @ 2015-11-03 13:44 UTC (permalink / raw)
To: Dan Carpenter, Somnath.Kotur-idTK6quXuVSLFuii7jzJGg
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
On 11/3/2015 3:11 PM, Dan Carpenter wrote:
> Hello Matan Barak,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch dbf727de7440: "IB/core: Use GID table in AH creation and
> dmac resolution" from Oct 15, 2015, leads to the following Smatch
> complaint:
>
> drivers/infiniband/hw/ocrdma/ocrdma_ah.c:157 ocrdma_create_ah()
> error: we previously assumed 'sgid_attr.ndev' could be null (see line 146)
>
> drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> 145 }
> 146 if (sgid_attr.ndev) {
> ^^^^^^^^^^^^^^
> Patch introduces a NULL check.
>
> 147 if (is_vlan_dev(sgid_attr.ndev))
> 148 vlan_tag = vlan_dev_vlan_id(sgid_attr.ndev);
> 149 dev_put(sgid_attr.ndev);
> 150 }
> 151
> 152 if ((pd->uctx) &&
> 153 (!rdma_is_multicast_addr((struct in6_addr *)attr->grh.dgid.raw)) &&
> 154 (!rdma_link_local_addr((struct in6_addr *)attr->grh.dgid.raw))) {
> 155 status = rdma_addr_find_dmac_by_grh(&sgid, &attr->grh.dgid,
> 156 attr->dmac, &vlan_tag,
> 157 sgid_attr.ndev->ifindex);
> ^^^^^^^^^^^^^^^^
> Patch introduces this new dereference. The warning might be a false
> positive if "pd->uctx" or rdma_is_multicast_addr() imply it's non-NULL
> but I don't know this code well enough to say for sure. Hence this
> email. :)
>
> 158 if (status) {
> 159 pr_err("%s(): Failed to resolve dmac from gid."
>
> regards,
> dan carpenter
>
Thanks for the catch Dan.
As I wrote in the commit message - "ocrdma driver changes were done by
Somnath Kotur <Somnath.Kotur-idTK6quXuVSLFuii7jzJGg@public.gmane.org>"
Somnath, RoCE implies non-NULL ndev, but dereferencing ifindex after
dev_put doesn't seem to be safe.
Could you please take a look?
Thanks,
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: IB/core: Use GID table in AH creation and dmac resolution
[not found] ` <CAOBf=msvVXCw129URdiWTLogGOpAjzboCB=h0haTh4wJZG5XXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-11-04 4:28 ` Somnath Kotur
0 siblings, 0 replies; 3+ messages in thread
From: Somnath Kotur @ 2015-11-04 4:28 UTC (permalink / raw)
To: Matan Barak; +Cc: Dan Carpenter, linux-rdma-u79uwXL29TY76Z2rM5mHXA
Thanks Dan and Matan.
We will take a look and revert on this
Thanks
Som
On Wed, Nov 4, 2015 at 9:31 AM, Somnath Kotur
<somnath.kotur-1wcpHE2jlwO1Z/+hSey0Gg@public.gmane.org> wrote:
> Thanks Dan and Matan.
>
> We will take a look and revert on this
>
> Thanks
> Som
>
> On Tue, Nov 3, 2015 at 7:14 PM, Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>>
>>
>>
>> On 11/3/2015 3:11 PM, Dan Carpenter wrote:
>>>
>>> Hello Matan Barak,
>>>
>>> This is a semi-automatic email about new static checker warnings.
>>>
>>> The patch dbf727de7440: "IB/core: Use GID table in AH creation and
>>> dmac resolution" from Oct 15, 2015, leads to the following Smatch
>>> complaint:
>>>
>>> drivers/infiniband/hw/ocrdma/ocrdma_ah.c:157 ocrdma_create_ah()
>>> error: we previously assumed 'sgid_attr.ndev' could be null (see
>>> line 146)
>>>
>>> drivers/infiniband/hw/ocrdma/ocrdma_ah.c
>>> 145 }
>>> 146 if (sgid_attr.ndev) {
>>> ^^^^^^^^^^^^^^
>>> Patch introduces a NULL check.
>>>
>>> 147 if (is_vlan_dev(sgid_attr.ndev))
>>> 148 vlan_tag =
>>> vlan_dev_vlan_id(sgid_attr.ndev);
>>> 149 dev_put(sgid_attr.ndev);
>>> 150 }
>>> 151
>>> 152 if ((pd->uctx) &&
>>> 153 (!rdma_is_multicast_addr((struct in6_addr
>>> *)attr->grh.dgid.raw)) &&
>>> 154 (!rdma_link_local_addr((struct in6_addr
>>> *)attr->grh.dgid.raw))) {
>>> 155 status = rdma_addr_find_dmac_by_grh(&sgid,
>>> &attr->grh.dgid,
>>> 156 attr->dmac,
>>> &vlan_tag,
>>> 157
>>> sgid_attr.ndev->ifindex);
>>>
>>> ^^^^^^^^^^^^^^^^
>>> Patch introduces this new dereference. The warning might be a false
>>> positive if "pd->uctx" or rdma_is_multicast_addr() imply it's non-NULL
>>> but I don't know this code well enough to say for sure. Hence this
>>> email. :)
>>>
>>> 158 if (status) {
>>> 159 pr_err("%s(): Failed to resolve dmac from
>>> gid."
>>>
>>> regards,
>>> dan carpenter
>>>
>>
>> Thanks for the catch Dan.
>> As I wrote in the commit message - "ocrdma driver changes were done by
>> Somnath Kotur <Somnath.Kotur-idTK6quXuVSLFuii7jzJGg@public.gmane.org>"
>> Somnath, RoCE implies non-NULL ndev, but dereferencing ifindex after
>> dev_put doesn't seem to be safe.
>> Could you please take a look?
>>
>> Thanks,
>> Matan
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-11-04 4:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-03 13:11 IB/core: Use GID table in AH creation and dmac resolution Dan Carpenter
2015-11-03 13:44 ` Matan Barak
[not found] ` <CAOBf=msvVXCw129URdiWTLogGOpAjzboCB=h0haTh4wJZG5XXw@mail.gmail.com>
[not found] ` <CAOBf=msvVXCw129URdiWTLogGOpAjzboCB=h0haTh4wJZG5XXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-11-04 4:28 ` Somnath Kotur
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).