* [PATCH net] cxgb4: Fix panic when attaching to ULD fails
@ 2019-10-14 7:50 Vishal Kulkarni
2019-10-15 5:52 ` Simon Horman
2019-10-16 19:19 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Vishal Kulkarni @ 2019-10-14 7:50 UTC (permalink / raw
To: netdev, davem; +Cc: nirranjan, Vishal Kulkarni, Shahjada Abul Husain
Release resources when attaching to ULD fail. Otherwise, data
mismatch is seen between LLD and ULD later on, which lead to
kernel panic when accessing resources that should not even
exist in the first place.
Fixes: 94cdb8bb993a ("cxgb4: Add support for dynamic allocation of resources for ULD")
Signed-off-by: Shahjada Abul Husain <shahjada@chelsio.com>
Signed-off-by: Vishal Kulkarni <vishal@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
index 5b60224..0482ef8 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
@@ -692,10 +692,10 @@ static void uld_init(struct adapter *adap, struct cxgb4_lld_info *lld)
lld->write_cmpl_support = adap->params.write_cmpl_support;
}
-static void uld_attach(struct adapter *adap, unsigned int uld)
+static int uld_attach(struct adapter *adap, unsigned int uld)
{
- void *handle;
struct cxgb4_lld_info lli;
+ void *handle;
uld_init(adap, &lli);
uld_queue_init(adap, uld, &lli);
@@ -705,7 +705,7 @@ static void uld_attach(struct adapter *adap, unsigned int uld)
dev_warn(adap->pdev_dev,
"could not attach to the %s driver, error %ld\n",
adap->uld[uld].name, PTR_ERR(handle));
- return;
+ return PTR_ERR(handle);
}
adap->uld[uld].handle = handle;
@@ -713,6 +713,8 @@ static void uld_attach(struct adapter *adap, unsigned int uld)
if (adap->flags & CXGB4_FULL_INIT_DONE)
adap->uld[uld].state_change(handle, CXGB4_STATE_UP);
+
+ return 0;
}
/**
@@ -727,8 +729,8 @@ static void uld_attach(struct adapter *adap, unsigned int uld)
void cxgb4_register_uld(enum cxgb4_uld type,
const struct cxgb4_uld_info *p)
{
- int ret = 0;
struct adapter *adap;
+ int ret = 0;
if (type >= CXGB4_ULD_MAX)
return;
@@ -760,7 +762,9 @@ void cxgb4_register_uld(enum cxgb4_uld type,
if (ret)
goto free_irq;
adap->uld[type] = *p;
- uld_attach(adap, type);
+ ret = uld_attach(adap, type);
+ if (ret)
+ goto free_irq;
continue;
free_irq:
if (adap->flags & CXGB4_FULL_INIT_DONE)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] cxgb4: Fix panic when attaching to ULD fails
2019-10-14 7:50 [PATCH net] cxgb4: Fix panic when attaching to ULD fails Vishal Kulkarni
@ 2019-10-15 5:52 ` Simon Horman
2019-10-16 19:19 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2019-10-15 5:52 UTC (permalink / raw
To: Vishal Kulkarni; +Cc: netdev, davem, nirranjan, Shahjada Abul Husain
On Mon, Oct 14, 2019 at 01:20:35PM +0530, Vishal Kulkarni wrote:
> Release resources when attaching to ULD fail. Otherwise, data
> mismatch is seen between LLD and ULD later on, which lead to
> kernel panic when accessing resources that should not even
> exist in the first place.
>
> Fixes: 94cdb8bb993a ("cxgb4: Add support for dynamic allocation of resources for ULD")
> Signed-off-by: Shahjada Abul Husain <shahjada@chelsio.com>
> Signed-off-by: Vishal Kulkarni <vishal@chelsio.com>
> ---
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
> index 5b60224..0482ef8 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
> @@ -692,10 +692,10 @@ static void uld_init(struct adapter *adap, struct cxgb4_lld_info *lld)
> lld->write_cmpl_support = adap->params.write_cmpl_support;
> }
>
> -static void uld_attach(struct adapter *adap, unsigned int uld)
> +static int uld_attach(struct adapter *adap, unsigned int uld)
> {
> - void *handle;
> struct cxgb4_lld_info lli;
> + void *handle;
>
> uld_init(adap, &lli);
> uld_queue_init(adap, uld, &lli);
> @@ -705,7 +705,7 @@ static void uld_attach(struct adapter *adap, unsigned int uld)
> dev_warn(adap->pdev_dev,
> "could not attach to the %s driver, error %ld\n",
> adap->uld[uld].name, PTR_ERR(handle));
> - return;
> + return PTR_ERR(handle);
> }
>
> adap->uld[uld].handle = handle;
> @@ -713,6 +713,8 @@ static void uld_attach(struct adapter *adap, unsigned int uld)
>
> if (adap->flags & CXGB4_FULL_INIT_DONE)
> adap->uld[uld].state_change(handle, CXGB4_STATE_UP);
> +
> + return 0;
> }
>
> /**
> @@ -727,8 +729,8 @@ static void uld_attach(struct adapter *adap, unsigned int uld)
> void cxgb4_register_uld(enum cxgb4_uld type,
> const struct cxgb4_uld_info *p)
Not part of this patch, but the comment above this function describes
it as returning -EBUSY and yet the return type is void. Also, the comment
seems to be in semi-kdoc format, perhaps converting it would be worthwhile.
> {
> - int ret = 0;
> struct adapter *adap;
> + int ret = 0;
>
> if (type >= CXGB4_ULD_MAX)
> return;
> @@ -760,7 +762,9 @@ void cxgb4_register_uld(enum cxgb4_uld type,
> if (ret)
> goto free_irq;
> adap->uld[type] = *p;
> - uld_attach(adap, type);
> + ret = uld_attach(adap, type);
> + if (ret)
> + goto free_irq;
Is it desired that the loop continues and that only the current iteration
is cleaned up?
> continue;
> free_irq:
> if (adap->flags & CXGB4_FULL_INIT_DONE)
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] cxgb4: Fix panic when attaching to ULD fails
2019-10-14 7:50 [PATCH net] cxgb4: Fix panic when attaching to ULD fails Vishal Kulkarni
2019-10-15 5:52 ` Simon Horman
@ 2019-10-16 19:19 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2019-10-16 19:19 UTC (permalink / raw
To: vishal; +Cc: netdev, nirranjan, shahjada
From: Vishal Kulkarni <vishal@chelsio.com>
Date: Mon, 14 Oct 2019 13:20:35 +0530
> @@ -760,7 +762,9 @@ void cxgb4_register_uld(enum cxgb4_uld type,
> if (ret)
> goto free_irq;
> adap->uld[type] = *p;
> - uld_attach(adap, type);
> + ret = uld_attach(adap, type);
> + if (ret)
> + goto free_irq;
You're not freeing up all of the txq_info ULD stuff that setup_sge_txq_uld
created.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-16 19:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-14 7:50 [PATCH net] cxgb4: Fix panic when attaching to ULD fails Vishal Kulkarni
2019-10-15 5:52 ` Simon Horman
2019-10-16 19:19 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.