| Message ID | 20221121154339.4088935-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5f58cad1e4c65bebee34292696c6d2105eeb2027 |
| Headers | show |
| Series | [v2] ocxl: fix pci device refcount leak when calling get_function_0() | expand |
On Mon, 2022-11-21 at 23:43 +0800, Yang Yingliang wrote: > get_function_0() calls pci_get_domain_bus_and_slot(), as comment > says, it returns a pci device with refcount increment, so after > using it, pci_dev_put() needs be called. > > Get the device reference when get_function_0() is not called, so > pci_dev_put() can be called in the error path and callers > unconditionally. And add comment above get_dvsec_vendor0() to tell > callers to call pci_dev_put(). > > Fixes: 87db7579ebd5 ("ocxl: control via sysfs whether the FPGA is > reloaded on a link reset") > Suggested-by: Andrew Donnellan <ajd@linux.ibm.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> Looks good! Acked-by: Andrew Donnellan <ajd@linux.ibm.com> > --- > v1 -> v2: > Add comment above get_dvsec_vendor0(). > Get reference where dev is function 0, and call pci_dev_put() > unconditionally in callers. > --- > drivers/misc/ocxl/config.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c > index e401a51596b9..92ab49705f64 100644 > --- a/drivers/misc/ocxl/config.c > +++ b/drivers/misc/ocxl/config.c > @@ -193,6 +193,18 @@ static int read_dvsec_vendor(struct pci_dev > *dev) > return 0; > } > > +/** > + * get_dvsec_vendor0() - Find a related PCI device (function 0) > + * @dev: PCI device to match > + * @dev0: The PCI device (function 0) found > + * @out_pos: The position of PCI device (function 0) > + * > + * Returns 0 on success, negative on failure. > + * > + * NOTE: If it's successful, the reference of dev0 is increased, > + * so after using it, the callers must call pci_dev_put() to give > + * up the reference. > + */ > static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev > **dev0, > int *out_pos) > { > @@ -202,10 +214,14 @@ static int get_dvsec_vendor0(struct pci_dev > *dev, struct pci_dev **dev0, > dev = get_function_0(dev); > if (!dev) > return -1; > + } else { > + dev = pci_dev_get(dev); > } > pos = find_dvsec(dev, OCXL_DVSEC_VENDOR_ID); > - if (!pos) > + if (!pos) { > + pci_dev_put(dev); > return -1; > + } > *dev0 = dev; > *out_pos = pos; > return 0; > @@ -222,6 +238,7 @@ int ocxl_config_get_reset_reload(struct pci_dev > *dev, int *val) > > pci_read_config_dword(dev0, pos + > OCXL_DVSEC_VENDOR_RESET_RELOAD, > &reset_reload); > + pci_dev_put(dev0); > *val = !!(reset_reload & BIT(0)); > return 0; > } > @@ -243,6 +260,7 @@ int ocxl_config_set_reset_reload(struct pci_dev > *dev, int val) > reset_reload &= ~BIT(0); > pci_write_config_dword(dev0, pos + > OCXL_DVSEC_VENDOR_RESET_RELOAD, > reset_reload); > + pci_dev_put(dev0); > return 0; > } >
On 21/11/2022 16:43, Yang Yingliang wrote: > get_function_0() calls pci_get_domain_bus_and_slot(), as comment > says, it returns a pci device with refcount increment, so after > using it, pci_dev_put() needs be called. > > Get the device reference when get_function_0() is not called, so > pci_dev_put() can be called in the error path and callers > unconditionally. And add comment above get_dvsec_vendor0() to tell > callers to call pci_dev_put(). > > Fixes: 87db7579ebd5 ("ocxl: control via sysfs whether the FPGA is reloaded on a link reset") > Suggested-by: Andrew Donnellan <ajd@linux.ibm.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- Thanks for fixing it! Acked-by: Frederic Barrat <fbarrat@linux.ibm.com> Fred > v1 -> v2: > Add comment above get_dvsec_vendor0(). > Get reference where dev is function 0, and call pci_dev_put() > unconditionally in callers. > --- > drivers/misc/ocxl/config.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c > index e401a51596b9..92ab49705f64 100644 > --- a/drivers/misc/ocxl/config.c > +++ b/drivers/misc/ocxl/config.c > @@ -193,6 +193,18 @@ static int read_dvsec_vendor(struct pci_dev *dev) > return 0; > } > > +/** > + * get_dvsec_vendor0() - Find a related PCI device (function 0) > + * @dev: PCI device to match > + * @dev0: The PCI device (function 0) found > + * @out_pos: The position of PCI device (function 0) > + * > + * Returns 0 on success, negative on failure. > + * > + * NOTE: If it's successful, the reference of dev0 is increased, > + * so after using it, the callers must call pci_dev_put() to give > + * up the reference. > + */ > static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev **dev0, > int *out_pos) > { > @@ -202,10 +214,14 @@ static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev **dev0, > dev = get_function_0(dev); > if (!dev) > return -1; > + } else { > + dev = pci_dev_get(dev); > } > pos = find_dvsec(dev, OCXL_DVSEC_VENDOR_ID); > - if (!pos) > + if (!pos) { > + pci_dev_put(dev); > return -1; > + } > *dev0 = dev; > *out_pos = pos; > return 0; > @@ -222,6 +238,7 @@ int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val) > > pci_read_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD, > &reset_reload); > + pci_dev_put(dev0); > *val = !!(reset_reload & BIT(0)); > return 0; > } > @@ -243,6 +260,7 @@ int ocxl_config_set_reset_reload(struct pci_dev *dev, int val) > reset_reload &= ~BIT(0); > pci_write_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD, > reset_reload); > + pci_dev_put(dev0); > return 0; > } >
On Mon, 21 Nov 2022 23:43:39 +0800, Yang Yingliang wrote: > get_function_0() calls pci_get_domain_bus_and_slot(), as comment > says, it returns a pci device with refcount increment, so after > using it, pci_dev_put() needs be called. > > Get the device reference when get_function_0() is not called, so > pci_dev_put() can be called in the error path and callers > unconditionally. And add comment above get_dvsec_vendor0() to tell > callers to call pci_dev_put(). > > [...] Applied to powerpc/next. [1/1] ocxl: fix pci device refcount leak when calling get_function_0() https://git.kernel.org/powerpc/c/5f58cad1e4c65bebee34292696c6d2105eeb2027 cheers
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c index e401a51596b9..92ab49705f64 100644 --- a/drivers/misc/ocxl/config.c +++ b/drivers/misc/ocxl/config.c @@ -193,6 +193,18 @@ static int read_dvsec_vendor(struct pci_dev *dev) return 0; } +/** + * get_dvsec_vendor0() - Find a related PCI device (function 0) + * @dev: PCI device to match + * @dev0: The PCI device (function 0) found + * @out_pos: The position of PCI device (function 0) + * + * Returns 0 on success, negative on failure. + * + * NOTE: If it's successful, the reference of dev0 is increased, + * so after using it, the callers must call pci_dev_put() to give + * up the reference. + */ static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev **dev0, int *out_pos) { @@ -202,10 +214,14 @@ static int get_dvsec_vendor0(struct pci_dev *dev, struct pci_dev **dev0, dev = get_function_0(dev); if (!dev) return -1; + } else { + dev = pci_dev_get(dev); } pos = find_dvsec(dev, OCXL_DVSEC_VENDOR_ID); - if (!pos) + if (!pos) { + pci_dev_put(dev); return -1; + } *dev0 = dev; *out_pos = pos; return 0; @@ -222,6 +238,7 @@ int ocxl_config_get_reset_reload(struct pci_dev *dev, int *val) pci_read_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD, &reset_reload); + pci_dev_put(dev0); *val = !!(reset_reload & BIT(0)); return 0; } @@ -243,6 +260,7 @@ int ocxl_config_set_reset_reload(struct pci_dev *dev, int val) reset_reload &= ~BIT(0); pci_write_config_dword(dev0, pos + OCXL_DVSEC_VENDOR_RESET_RELOAD, reset_reload); + pci_dev_put(dev0); return 0; }
get_function_0() calls pci_get_domain_bus_and_slot(), as comment says, it returns a pci device with refcount increment, so after using it, pci_dev_put() needs be called. Get the device reference when get_function_0() is not called, so pci_dev_put() can be called in the error path and callers unconditionally. And add comment above get_dvsec_vendor0() to tell callers to call pci_dev_put(). Fixes: 87db7579ebd5 ("ocxl: control via sysfs whether the FPGA is reloaded on a link reset") Suggested-by: Andrew Donnellan <ajd@linux.ibm.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- v1 -> v2: Add comment above get_dvsec_vendor0(). Get reference where dev is function 0, and call pci_dev_put() unconditionally in callers. --- drivers/misc/ocxl/config.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)