netdev - RE: [PATCH v2] ixgbe: check adapter->vfinfo before dereference

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87618083B2453E4A8714035B62D67992500E19F5@FMSMSX105.amr.corp.intel.com>
Date:	Tue, 14 Oct 2014 23:18:35 +0000
From:	"Tantilov, Emil S" <emil.s.tantilov@...el.com>
To:	Thierry Herbelot <thierry.herbelot@...nd.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH v2] ixgbe: check adapter->vfinfo before dereference

>-----Original Message-----
>From: netdev-owner@...r.kernel.org [mailto:netdev-
>owner@...r.kernel.org] On Behalf Of Thierry Herbelot
>Sent: Friday, October 10, 2014 1:46 AM
>To: Kirsher, Jeffrey T; Brandeburg, Jesse; Allan, Bruce W;
>netdev@...r.kernel.org
>Cc: Thierry Herbelot
>Subject: [PATCH v2] ixgbe: check adapter->vfinfo before
>dereference
>
>this protects against the following panic:
>(before a VF was actually created on p96p1 PF Ethernet port)
>ip link set p96p1 vf 0 spoofchk off
>BUG: unable to handle kernel NULL pointer dereference at
>0000000000000052
>IP: [<ffffffffa044a1c1>]
>ixgbe_ndo_set_vf_spoofchk+0x51/0x150 [ixgbe]
>
>Signed-off-by: Thierry Herbelot <thierry.herbelot@...nd.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |   73
>+++++++++++++++++++++++-
> 1 file changed, 70 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>index 706fc69..29279ad 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>@@ -316,7 +316,7 @@ static int ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
> 	int entries = (msgbuf[0] & IXGBE_VT_MSGINFO_MASK)
> 		       >> IXGBE_VT_MSGINFO_SHIFT;
> 	u16 *hash_list = (u16 *)&msgbuf[1];
>-	struct vf_data_storage *vfinfo = &adapter->vfinfo[vf];
>+	struct vf_data_storage *vfinfo;
> 	struct ixgbe_hw *hw = &adapter->hw;
> 	int i;
> 	u32 vector_bit;
>@@ -324,6 +324,11 @@ static int
>ixgbe_set_vf_multicasts(struct ixgbe_adapter *adapter,
> 	u32 mta_reg;
> 	u32 vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
>
>+	if (!adapter->vfinfo)
>+		return -1;
>+
>+	vfinfo = &adapter->vfinfo[vf];

This check makes sense for the ndo functions that get called by the ip command, but I don't think we need to add them before every use of adapter->vfinfo. In this case for example ixgbe_set_vf_multicasts() is called from
ixgbe_rcv_msg_from_vf() which will be called when an actual VF exists.

Also for the error -EINVAL probably makes more sense than -1.

Thanks,
Emil


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html