2023-09-19 14:45:40

by Wen Gu

Subject: [PATCH net-next 01/18] net/smc: decouple ism_dev from SMC-D device dump

This patch helps to decouple ISM device from SMC-D device, allowing
different underlying device forms, such as virtual ISM devices.

Signed-off-by: Wen Gu <[email protected]>
---
net/smc/smc_ism.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index fbee249..0045fee 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -230,12 +230,11 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
char smc_pnet[SMC_MAX_PNETID_LEN + 1];
struct smc_pci_dev smc_pci_dev;
struct nlattr *port_attrs;
+ struct device *priv_dev;
struct nlattr *attrs;
- struct ism_dev *ism;
int use_cnt = 0;
void *nlh;

- ism = smcd->priv;
nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
&smc_gen_nl_family, NLM_F_MULTI,
SMC_NETLINK_GET_DEV_SMCD);
@@ -250,7 +249,10 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
if (nla_put_u8(skb, SMC_NLA_DEV_IS_CRIT, use_cnt > 0))
goto errattr;
memset(&smc_pci_dev, 0, sizeof(smc_pci_dev));
- smc_set_pci_values(to_pci_dev(ism->dev.parent), &smc_pci_dev);
+ if (smcd->ops->get_dev)
+ priv_dev = smcd->ops->get_dev(smcd);
+ if (priv_dev->parent)
+ smc_set_pci_values(to_pci_dev(priv_dev->parent), &smc_pci_dev);
if (nla_put_u32(skb, SMC_NLA_DEV_PCI_FID, smc_pci_dev.pci_fid))
goto errattr;
if (nla_put_u16(skb, SMC_NLA_DEV_PCI_CHID, smc_pci_dev.pci_pchid))
--
1.8.3.1

2023-09-21 22:13:17

by Simon Horman

Subject: Re: [PATCH net-next 01/18] net/smc: decouple ism_dev from SMC-D device dump

On Tue, Sep 19, 2023 at 10:41:45PM +0800, Wen Gu wrote:
> This patch helps to decouple ISM device from SMC-D device, allowing
> different underlying device forms, such as virtual ISM devices.
>
> Signed-off-by: Wen Gu <[email protected]>
> ---
> net/smc/smc_ism.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
> index fbee249..0045fee 100644
> --- a/net/smc/smc_ism.c
> +++ b/net/smc/smc_ism.c
> @@ -230,12 +230,11 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
> char smc_pnet[SMC_MAX_PNETID_LEN + 1];
> struct smc_pci_dev smc_pci_dev;
> struct nlattr *port_attrs;
> + struct device *priv_dev;
> struct nlattr *attrs;
> - struct ism_dev *ism;
> int use_cnt = 0;
> void *nlh;
>
> - ism = smcd->priv;
> nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
> &smc_gen_nl_family, NLM_F_MULTI,
> SMC_NETLINK_GET_DEV_SMCD);
> @@ -250,7 +249,10 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
> if (nla_put_u8(skb, SMC_NLA_DEV_IS_CRIT, use_cnt > 0))
> goto errattr;
> memset(&smc_pci_dev, 0, sizeof(smc_pci_dev));

Hi Wen Gu,

priv_dev is uninitialised here.

> - smc_set_pci_values(to_pci_dev(ism->dev.parent), &smc_pci_dev);
> + if (smcd->ops->get_dev)
> + priv_dev = smcd->ops->get_dev(smcd);

It is conditionally initialised here.

> + if (priv_dev->parent)

But unconditionally dereferenced here.

As flagged by clang-16 W=1, and Smatch

> + smc_set_pci_values(to_pci_dev(priv_dev->parent), &smc_pci_dev);
> if (nla_put_u32(skb, SMC_NLA_DEV_PCI_FID, smc_pci_dev.pci_fid))
> goto errattr;
> if (nla_put_u16(skb, SMC_NLA_DEV_PCI_CHID, smc_pci_dev.pci_pchid))
> --
> 1.8.3.1
>
>

2023-09-22 08:54:01

by Wen Gu

Subject: Re: [PATCH net-next 01/18] net/smc: decouple ism_dev from SMC-D device dump

On 2023/9/22 04:41, Simon Horman wrote:
> On Tue, Sep 19, 2023 at 10:41:45PM +0800, Wen Gu wrote:
>> This patch helps to decouple ISM device from SMC-D device, allowing
>> different underlying device forms, such as virtual ISM devices.
>>
>> Signed-off-by: Wen Gu <[email protected]>
>> ---
>> net/smc/smc_ism.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
>> index fbee249..0045fee 100644
>> --- a/net/smc/smc_ism.c
>> +++ b/net/smc/smc_ism.c
>> @@ -230,12 +230,11 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
>> char smc_pnet[SMC_MAX_PNETID_LEN + 1];
>> struct smc_pci_dev smc_pci_dev;
>> struct nlattr *port_attrs;
>> + struct device *priv_dev;
>> struct nlattr *attrs;
>> - struct ism_dev *ism;
>> int use_cnt = 0;
>> void *nlh;
>>
>> - ism = smcd->priv;
>> nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
>> &smc_gen_nl_family, NLM_F_MULTI,
>> SMC_NETLINK_GET_DEV_SMCD);
>> @@ -250,7 +249,10 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
>> if (nla_put_u8(skb, SMC_NLA_DEV_IS_CRIT, use_cnt > 0))
>> goto errattr;
>> memset(&smc_pci_dev, 0, sizeof(smc_pci_dev));
>
> Hi Wen Gu,
>
> priv_dev is uninitialised here.
>
>> - smc_set_pci_values(to_pci_dev(ism->dev.parent), &smc_pci_dev);
>> + if (smcd->ops->get_dev)
>> + priv_dev = smcd->ops->get_dev(smcd);
>
> It is conditionally initialised here.
>
>> + if (priv_dev->parent)
>
> But unconditionally dereferenced here.
>
> As flagged by clang-16 W=1, and Smatch
>

Hi Simon. Yes, I fixed it in v3. Thank you!

>> + smc_set_pci_values(to_pci_dev(priv_dev->parent), &smc_pci_dev);
>> if (nla_put_u32(skb, SMC_NLA_DEV_PCI_FID, smc_pci_dev.pci_fid))
>> goto errattr;
>> if (nla_put_u16(skb, SMC_NLA_DEV_PCI_CHID, smc_pci_dev.pci_pchid))
>> --
>> 1.8.3.1
>>
>>

2023-09-22 21:31:13

by Gerd Bayer

Subject: Re: [PATCH net-next 01/18] net/smc: decouple ism_dev from SMC-D device dump

On Fri, 2023-09-22 at 16:05 +0800, Wen Gu wrote:
> On 2023/9/22 04:41, Simon Horman wrote:
> > On Tue, Sep 19, 2023 at 10:41:45PM +0800, Wen Gu wrote:
> >
> > priv_dev is uninitialised here.
> >
> > > -       smc_set_pci_values(to_pci_dev(ism->dev.parent),
> > > &smc_pci_dev);
> > > +       if (smcd->ops->get_dev)
> > > +               priv_dev = smcd->ops->get_dev(smcd);
> >
> > It is conditionally initialised here.
> >
> > > +       if (priv_dev->parent)
> >
> > But unconditionally dereferenced here.
> >
> > As flagged by clang-16 W=1, and Smatch
> >
>
> Hi Simon. Yes, I fixed it in v3. Thank you!

Hi Wen Gu,

seems like there is some email filter at work. Neither v2 nor v3 made
it to the netdev mailing list - nor to patchwork.kernel.org.
There's traces of Wenjia's replies and your replies to her - but not
the original mail.

Could you please check? Thanks!
Gerd

2023-09-23 09:25:11

by Wen Gu

Subject: Re: [PATCH net-next 01/18] net/smc: decouple ism_dev from SMC-D device dump

On 2023/9/23 02:13, Gerd Bayer wrote:

> Hi Wen Gu,
>
> seems like there is some email filter at work. Neither v2 nor v3 made
> it to the netdev mailing list - nor to patchwork.kernel.org.
> There's traces of Wenjia's replies and your replies to her - but not
> the original mail.
>
> Could you please check? Thanks!
> Gerd

Yes, it is ture. v2 and v3 was refused by ver.kernel.org.

I will send the v4 based on Wenjia's comments as soon as possible,
and add CC of you, Sandy, Niklas and Halil in v4 in case the filter
happens again.

Thank you very much for your reminder!

Regards,
Wen Gu