| Message ID | 20241025093135.1053121-1-chenridong@huaweicloud.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | qed/qed_sriov: avoid null-ptr-deref | expand |
From: Chen Ridong <chenridong@huaweicloud.com> Date: Fri, 25 Oct 2024 09:31:35 +0000 > [PATCH] qed/qed_sriov: avoid null-ptr-deref Use the correct tree prefix, [PATCH net] in your case. > From: Chen Ridong <chenridong@huawei.com> Why do you commit from @huawei.com, but send from @huaweicloud.com? > > The qed_iov_get_public_vf_info may return NULL, which may lead to > null-ptr-deref. To avoid possible null-ptr-deref, check vf_info Do you have a repro for this or it's purely hypothetical? > before accessing its member. > Here you should have a "Fixes:" tag if you believe this is a fix. > Signed-off-by: Chen Ridong <chenridong@huawei.com> > --- > drivers/net/ethernet/qlogic/qed/qed_sriov.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c > index fa167b1aa019..30da9865496d 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c > @@ -2997,6 +2997,8 @@ static int qed_iov_pre_update_vport(struct qed_hwfn *hwfn, > return 0; > > vf_info = qed_iov_get_public_vf_info(hwfn, vfid, true); > + if (!vf_info) > + return 0; 0 or error code? > > if (flags->update_rx_mode_config) { > vf_info->rx_accept_mode = flags->rx_accept_filter; Thanks, Olek
On 2024/10/25 23:28, Alexander Lobakin wrote: > From: Chen Ridong <chenridong@huaweicloud.com> > Date: Fri, 25 Oct 2024 09:31:35 +0000 > >> [PATCH] qed/qed_sriov: avoid null-ptr-deref > > Use the correct tree prefix, [PATCH net] in your case. > Thanks, will update >> From: Chen Ridong <chenridong@huawei.com> > > Why do you commit from @huawei.com, but send from @huaweicloud.com? > The @huawei.com is the email I am actually using. But if I use it to send email, my patches may not appear in maintainers's inbox list. This won't be happened when I use 'huaweicloud.com' to send emails. So I am using 'huaweicloud.com' to communicate with community. However, I would like to maintain the same author identity. >> >> The qed_iov_get_public_vf_info may return NULL, which may lead to >> null-ptr-deref. To avoid possible null-ptr-deref, check vf_info > > Do you have a repro for this or it's purely hypothetical? > I read the code and found that calling qed_iov_get_public_vf_info without checking whether the 'vfid' is valid may result in a null pointer, which may lead to a null pointer dereference. >> before accessing its member. >> > > Here you should have a "Fixes:" tag if you believe this is a fix. > >> Signed-off-by: Chen Ridong <chenridong@huawei.com> >> --- >> drivers/net/ethernet/qlogic/qed/qed_sriov.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c >> index fa167b1aa019..30da9865496d 100644 >> --- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c >> +++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c >> @@ -2997,6 +2997,8 @@ static int qed_iov_pre_update_vport(struct qed_hwfn *hwfn, >> return 0; >> >> vf_info = qed_iov_get_public_vf_info(hwfn, vfid, true); >> + if (!vf_info) >> + return 0; > > 0 or error code? It should return error code after I read the code again. Thank you very much. > >> >> if (flags->update_rx_mode_config) { >> vf_info->rx_accept_mode = flags->rx_accept_filter; > > Thanks, > Olek > Best regards, Ridong
From: Chen Ridong <chenridong@huaweicloud.com> Date: Tue, 29 Oct 2024 09:42:11 +0800 > > > On 2024/10/25 23:28, Alexander Lobakin wrote: >> From: Chen Ridong <chenridong@huaweicloud.com> >> Date: Fri, 25 Oct 2024 09:31:35 +0000 >> >>> [PATCH] qed/qed_sriov: avoid null-ptr-deref >> >> Use the correct tree prefix, [PATCH net] in your case. >> > > Thanks, will update > >>> From: Chen Ridong <chenridong@huawei.com> >> >> Why do you commit from @huawei.com, but send from @huaweicloud.com? >> > The @huawei.com is the email I am actually using. But if I use it to > send email, my patches may not appear in maintainers's inbox list. This > won't be happened when I use 'huaweicloud.com' to send emails. So I am > using 'huaweicloud.com' to communicate with community. However, I would > like to maintain the same author identity. > >>> >>> The qed_iov_get_public_vf_info may return NULL, which may lead to >>> null-ptr-deref. To avoid possible null-ptr-deref, check vf_info >> >> Do you have a repro for this or it's purely hypothetical? >> > > I read the code and found that calling qed_iov_get_public_vf_info > without checking whether the 'vfid' is valid may result in a null > pointer, which may lead to a null pointer dereference. If you want to submit a fix, you need to have a step-by-step manual how to reproduce the bug you're fixing. > >>> before accessing its member. >>> >> >> Here you should have a "Fixes:" tag if you believe this is a fix. Thanks, Olek
On 2024/10/29 23:15, Alexander Lobakin wrote: > From: Chen Ridong <chenridong@huaweicloud.com> > Date: Tue, 29 Oct 2024 09:42:11 +0800 > >> >> >> On 2024/10/25 23:28, Alexander Lobakin wrote: >>> From: Chen Ridong <chenridong@huaweicloud.com> >>> Date: Fri, 25 Oct 2024 09:31:35 +0000 >>> >>>> [PATCH] qed/qed_sriov: avoid null-ptr-deref >>> >>> Use the correct tree prefix, [PATCH net] in your case. >>> >> >> Thanks, will update >> >>>> From: Chen Ridong <chenridong@huawei.com> >>> >>> Why do you commit from @huawei.com, but send from @huaweicloud.com? >>> >> The @huawei.com is the email I am actually using. But if I use it to >> send email, my patches may not appear in maintainers's inbox list. This >> won't be happened when I use 'huaweicloud.com' to send emails. So I am >> using 'huaweicloud.com' to communicate with community. However, I would >> like to maintain the same author identity. >> >>>> >>>> The qed_iov_get_public_vf_info may return NULL, which may lead to >>>> null-ptr-deref. To avoid possible null-ptr-deref, check vf_info >>> >>> Do you have a repro for this or it's purely hypothetical? >>> >> >> I read the code and found that calling qed_iov_get_public_vf_info >> without checking whether the 'vfid' is valid may result in a null >> pointer, which may lead to a null pointer dereference. > > If you want to submit a fix, you need to have a step-by-step manual how > to reproduce the bug you're fixing. > >> >>>> before accessing its member. >>>> >>> >>> Here you should have a "Fixes:" tag if you believe this is a fix. > > Thanks, > Olek Thanks, I will try to reproduce this bug. Best regards, Ridong
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.c b/drivers/net/ethernet/qlogic/qed/qed_sriov.c index fa167b1aa019..30da9865496d 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_sriov.c +++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.c @@ -2997,6 +2997,8 @@ static int qed_iov_pre_update_vport(struct qed_hwfn *hwfn, return 0; vf_info = qed_iov_get_public_vf_info(hwfn, vfid, true); + if (!vf_info) + return 0; if (flags->update_rx_mode_config) { vf_info->rx_accept_mode = flags->rx_accept_filter; @@ -5145,6 +5147,9 @@ static void qed_iov_handle_trust_change(struct qed_hwfn *hwfn) * needed. */ vf_info = qed_iov_get_public_vf_info(hwfn, i, true); + if (!vf_info) + continue; + if (vf_info->is_trusted_configured == vf_info->is_trusted_request) continue;