On 2024/4/9 17:26, Leon Romanovsky wrote: > On Tue, Apr 09, 2024 at 11:30:47AM +0300, Aleksandr Mishin wrote: >> In hns_roce_hw_v2_get_cfg() pci_match_id() may return >> NULL which is later dereferenced. Fix this bug by adding NULL check. > > I don't know, this NULL can't happen in this flow. > > Thanks > Yeah, it's already checked here: 6911 static int hns_roce_hw_v2_init_instance(struct hnae3_handle *handle) 6912 { 6913 const struct hnae3_ae_ops *ops = handle->ae_algo->ops; 6914 const struct pci_device_id *id; 6915 struct device *dev = &handle->pdev->dev; 6916 int ret; 6917 6918 handle->rinfo.instance_state = HNS_ROCE_STATE_INIT; 6919 6920 if (ops->ae_dev_resetting(handle) || ops->get_hw_reset_stat(handle)) { 6921 handle->rinfo.instance_state = HNS_ROCE_STATE_NON_INIT; 6922 goto reset_chk_err; 6923 } 6924 6925 id = pci_match_id(hns_roce_hw_v2_pci_tbl, handle->pdev); 6926 if (!id) 6927 return 0; 6928 6929 if (id->driver_data && handle->pdev->revision == PCI_REVISION_ID_HIP08) 6930 return 0; 6931 6932 ret = __hns_roce_hw_v2_init_instance(handle); Junxian >> >> Found by Linux Verification Center (linuxtesting.org) with SVACE. >> >> Fixes: 0b567cde9d7a ("RDMA/hns: Enable RoCE on virtual functions") >> Signed-off-by: Aleksandr Mishin <amishin@xxxxxxxxxx> >> --- >> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> index ba7ae792d279..31a2093334d9 100644 >> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c >> @@ -6754,7 +6754,7 @@ static const struct pci_device_id hns_roce_hw_v2_pci_tbl[] = { >> >> MODULE_DEVICE_TABLE(pci, hns_roce_hw_v2_pci_tbl); >> >> -static void hns_roce_hw_v2_get_cfg(struct hns_roce_dev *hr_dev, >> +static int hns_roce_hw_v2_get_cfg(struct hns_roce_dev *hr_dev, >> struct hnae3_handle *handle) >> { >> struct hns_roce_v2_priv *priv = hr_dev->priv; >> @@ -6763,6 +6763,9 @@ static void hns_roce_hw_v2_get_cfg(struct hns_roce_dev *hr_dev, >> >> hr_dev->pci_dev = handle->pdev; >> id = pci_match_id(hns_roce_hw_v2_pci_tbl, hr_dev->pci_dev); >> + if (!id) >> + return -ENXIO; >> + >> hr_dev->is_vf = id->driver_data; >> hr_dev->dev = &handle->pdev->dev; >> hr_dev->hw = &hns_roce_hw_v2; >> @@ -6789,6 +6792,8 @@ static void hns_roce_hw_v2_get_cfg(struct hns_roce_dev *hr_dev, >> >> hr_dev->reset_cnt = handle->ae_algo->ops->ae_dev_reset_cnt(handle); >> priv->handle = handle; >> + >> + return 0; >> } >> >> static int __hns_roce_hw_v2_init_instance(struct hnae3_handle *handle) >> @@ -6806,7 +6811,11 @@ static int __hns_roce_hw_v2_init_instance(struct hnae3_handle *handle) >> goto error_failed_kzalloc; >> } >> >> - hns_roce_hw_v2_get_cfg(hr_dev, handle); >> + ret = hns_roce_hw_v2_get_cfg(hr_dev, handle); >> + if (ret) { >> + dev_err(hr_dev->dev, "RoCE Engine cfg failed!\n"); >> + goto error_failed_roce_init; >> + } >> >> ret = hns_roce_init(hr_dev); >> if (ret) { >> -- >> 2.30.2 >>