Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Sudarsana Reddy Kalluru <skalluru@marvell.com>
To:     Ivan Mironov <mironov.ivan@gmail.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
CC:     Ariel Elior <ariel.elior@cavium.com>,
        Sudarsana Kalluru <sudarsana.kalluru@cavium.com>,
        "everest-linux-l2@cavium.com" <everest-linux-l2@cavium.com>,
        "David S. Miller" <davem@davemloft.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [EXT] [PATCH v2] bnx2x: Fix NULL pointer dereference in
 bnx2x_del_all_vlans() on some hw
Thread-Topic: [EXT] [PATCH v2] bnx2x: Fix NULL pointer dereference in
 bnx2x_del_all_vlans() on some hw
Thread-Index: AQHUm5s/L9O9kr6JuUiJqq8V86uG/qWODVTg
Date:   Mon, 24 Dec 2018 16:06:06 +0000
Message-ID: <CY4PR18MB1253E8A065AB1B9C2241D593D3BB0@CY4PR18MB1253.namprd18.prod.outlook.com>
References: <20181224151305.31027-1-mironov.ivan@gmail.com>
In-Reply-To: <20181224151305.31027-1-mironov.ivan@gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: marvell.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: b30d98da-544d-46a5-e4d0-08d669b9b641
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Dec 2018 16:06:06.5410
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR18MB1013
X-OriginatorOrg: marvell.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-12-24_09:,,
 signatures=0
X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501
 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0
 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0
 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.0.1-1810050000 definitions=main-1812240140
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


-----Original Message-----
From: Ivan Mironov [mailto:mironov.ivan@gmail.com]=20
Sent: 24 December 2018 20:43
To: netdev@vger.kernel.org
Cc: Ariel Elior <ariel.elior@cavium.com>; Sudarsana Kalluru <sudarsana.kall=
uru@cavium.com>; everest-linux-l2@cavium.com; David S. Miller <davem@daveml=
oft.net>; linux-kernel@vger.kernel.org; Sudarsana Reddy Kalluru <skalluru@m=
arvell.com>; Ivan Mironov <mironov.ivan@gmail.com>
Subject: [EXT] [PATCH v2] bnx2x: Fix NULL pointer dereference in bnx2x_del_=
all_vlans() on some hw

External Email

----------------------------------------------------------------------
This happened when I tried to boot normal Fedora 29 system with latest avai=
lable kernel (from fedora rawhide, plus some unrelated custom
patches):

	BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
	PGD 0 P4D 0
	Oops: 0010 [#1] SMP PTI
	CPU: 6 PID: 1422 Comm: libvirtd Tainted: G          I       4.20.0-0.rc7.g=
it3.hpsa2.1.fc29.x86_64 #1
	Hardware name: HP ProLiant BL460c G6, BIOS I24 05/21/2018
	RIP: 0010:          (null)
	Code: Bad RIP value.
	RSP: 0018:ffffa47ccdc9fbe0 EFLAGS: 00010246
	RAX: 0000000000000000 RBX: 00000000000003e8 RCX: ffffa47ccdc9fbf8
	RDX: ffffa47ccdc9fc00 RSI: ffff97d9ee7b01f8 RDI: ffff97d9f0150b80
	RBP: ffff97d9f0150b80 R08: 0000000000000000 R09: 0000000000000000
	R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000003
	R13: ffff97d9ef1e53e8 R14: 0000000000000009 R15: ffff97d9f0ac6730
	FS:  00007f4d224ef700(0000) GS:ffff97d9fa200000(0000) knlGS:00000000000000=
00
	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
	CR2: ffffffffffffffd6 CR3: 00000011ece52006 CR4: 00000000000206e0
	Call Trace:
	 ? bnx2x_chip_cleanup+0x195/0x610 [bnx2x]
	 ? bnx2x_nic_unload+0x1e2/0x8f0 [bnx2x]
	 ? bnx2x_reload_if_running+0x24/0x40 [bnx2x]
	 ? bnx2x_set_features+0x79/0xa0 [bnx2x]
	 ? __netdev_update_features+0x244/0x9e0
	 ? netlink_broadcast_filtered+0x136/0x4b0
	 ? netdev_update_features+0x22/0x60
	 ? dev_disable_lro+0x1c/0xe0
	 ? devinet_sysctl_forward+0x1c6/0x211
	 ? proc_sys_call_handler+0xab/0x100
	 ? __vfs_write+0x36/0x1a0
	 ? rcu_read_lock_sched_held+0x79/0x80
	 ? rcu_sync_lockdep_assert+0x2e/0x60
	 ? __sb_start_write+0x14c/0x1b0
	 ? vfs_write+0x159/0x1c0
	 ? vfs_write+0xba/0x1c0
	 ? ksys_write+0x52/0xc0
	 ? do_syscall_64+0x60/0x1f0
	 ? entry_SYSCALL_64_after_hwframe+0x49/0xbe

After some investigation I figured out that recently added cleanup code tri=
es to call VLAN filtering de-initialization function which exist only for n=
ewer hardware. Corresponding function pointer is not set (=3D=3D 0) for old=
er hardware, namely these chips:

	#define CHIP_NUM_57710			0x164e
	#define CHIP_NUM_57711			0x164f
	#define CHIP_NUM_57711E			0x1650

And I have one of those in my test system:

	Broadcom Inc. and subsidiaries NetXtreme II BCM57711E 10-Gigabit PCIe [14e=
4:1650]

Function bnx2x_init_vlan_mac_fp_objs() from drivers/net/ethernet/broadcom/b=
nx2x/bnx2x_cmn.h decides whether to initialize relevant pointers in bnx2x_s=
p_objs.vlan_obj or not.

This regression was introduced after v4.20-rc7, and still exists in v4.20 r=
elease.

Fixes: 04f05230c5c13 ("bnx2x: Remove configured vlans as part of unload seq=
uence.")
Signed-off-by: Ivan Mironov <mironov.ivan@gmail.com>
---
v2:
- As suggested by Sudarsana Reddy Kalluru, do not call bnx2x_del_all_vlans(=
) at
  all if (!CHIP_IS_E1x(bp)).
v1:
- Check for chip num instead of (vlan_obj->delete_all !=3D 0).
v0:
- Patch introduced.
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net=
/ethernet/broadcom/bnx2x/bnx2x_main.c
index b164f705709d..3b5b47e98c73 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9360,10 +9360,16 @@ void bnx2x_chip_cleanup(struct bnx2x *bp, int unloa=
d_mode, bool keep_link)
 		BNX2X_ERR("Failed to schedule DEL commands for UC MACs list: %d\n",
 			  rc);
=20
-	/* Remove all currently configured VLANs */
-	rc =3D bnx2x_del_all_vlans(bp);
-	if (rc < 0)
-		BNX2X_ERR("Failed to delete all VLANs\n");
+	/* The whole *vlan_obj structure may be not initialized if VLAN
+	 * filtering offload is not supported by hardware. Currently this is
+	 * true for all hardware covered by CHIP_IS_E1x().
+	 */
+	if (!CHIP_IS_E1x(bp)) {
+		/* Remove all currently configured VLANs */
+		rc =3D bnx2x_del_all_vlans(bp);
+		if (rc < 0)
+			BNX2X_ERR("Failed to delete all VLANs\n");
+	}
=20
 	/* Disable LLH */
 	if (!CHIP_IS_E1(bp))
--
2.20.1

Thanks for the changes.
Acked-by: Sudarsana Kalluru <Sudarsana.Kalluru@cavium.com>