lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 22 Dec 2022 10:57:32 +0100 From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com> To: Daniil Tatianin <d-tatianin@...dex-team.ru> Cc: Shahed Shaikh <shshaikh@...vell.com>, Manish Chopra <manishc@...vell.com>, GR-Linux-NIC-Dev@...vell.com, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net v2] qlcnic: prevent ->dcb use-after-free on qlcnic_dcb_enable() failure On Thu, Dec 22, 2022 at 10:42:23AM +0300, Daniil Tatianin wrote: > adapter->dcb would get silently freed inside qlcnic_dcb_enable() in > case qlcnic_dcb_attach() would return an error, which always happens > under OOM conditions. This would lead to use-after-free because both > of the existing callers invoke qlcnic_dcb_get_info() on the obtained > pointer, which is potentially freed at that point. > > Propagate errors from qlcnic_dcb_enable(), and instead free the dcb > pointer at callsite using qlcnic_dcb_free(). This also removes the now > unused qlcnic_clear_dcb_ops() helper, which was a simple wrapper around > kfree() also causing memory leaks for partially initialized dcb. > > Found by Linux Verification Center (linuxtesting.org) with the SVACE > static analysis tool. > > Fixes: 3c44bba1d270 ("qlcnic: Disable DCB operations from SR-IOV VFs") > Signed-off-by: Daniil Tatianin <d-tatianin@...dex-team.ru> > --- > Changes since v1: > - Add a fixes tag + net as a target > - Remove qlcnic_clear_dcb_ops entirely & use qlcnic_dcb_free instead > --- > drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c | 9 ++++++++- > drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.h | 10 ++-------- > drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 9 ++++++++- > 3 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c > index dbb800769cb6..774f2c6875ec 100644 > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c > @@ -2505,7 +2505,14 @@ int qlcnic_83xx_init(struct qlcnic_adapter *adapter) > goto disable_mbx_intr; > > qlcnic_83xx_clear_function_resources(adapter); > - qlcnic_dcb_enable(adapter->dcb); > + > + err = qlcnic_dcb_enable(adapter->dcb); > + if (err) { > + qlcnic_dcb_free(adapter->dcb); > + adapter->dcb = NULL; Small nit, I think qlcnic_dcb_free() already set adapter->dcb to NULL. Otherwise, thanks for changing: Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com> Thanks, Michal [...] > -- > 2.25.1
Powered by blists - more mailing lists