From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 66059C46467 for ; Thu, 19 Jan 2023 15:18:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230344AbjASPSD (ORCPT ); Thu, 19 Jan 2023 10:18:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229683AbjASPR6 (ORCPT ); Thu, 19 Jan 2023 10:17:58 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0CBE882996 for ; Thu, 19 Jan 2023 07:17:57 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9E3A261B53 for ; Thu, 19 Jan 2023 15:17:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AAAA2C433F0; Thu, 19 Jan 2023 15:17:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1674141476; bh=d+RJax+gn4iXgWIKUImZxLaGkCJ5L1Iboau8yyzzzoo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RstmJ0otBERnHgKaLfZmAkGg1CCZYd56wwtN40ndpz9lrPOBCMIOFK0CJYHfOmfso jc3mcT/BcLQShASUz3WUg/Z3vIokxDEnAY70iKfutYE4SC/aE7ClvHFkairLaXiNX8 pB3vIyr8m2N79vEUrntQW9d/xfLWYhM83obdtkZg= Date: Thu, 19 Jan 2023 16:17:48 +0100 From: Greg Kroah-Hartman To: Alexander Shishkin Cc: mst@redhat.com, jasowang@redhat.com, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, elena.reshetova@intel.com, kirill.shutemov@linux.intel.com, Andi Kleen , Amit Shah , Arnd Bergmann Subject: Re: [PATCH v1 1/6] virtio console: Harden multiport against invalid host input Message-ID: References: <20230119135721.83345-1-alexander.shishkin@linux.intel.com> <20230119135721.83345-2-alexander.shishkin@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230119135721.83345-2-alexander.shishkin@linux.intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 19, 2023 at 03:57:16PM +0200, Alexander Shishkin wrote: > From: Andi Kleen > > It's possible for the host to set the multiport flag, but pass in > 0 multiports, which results in: > > BUG: KASAN: slab-out-of-bounds in init_vqs+0x244/0x6c0 drivers/char/virtio_console.c:1878 > Write of size 8 at addr ffff888001cc24a0 by task swapper/1 > > CPU: 0 PID: 1 Comm: swapper Not tainted 5.15.0-rc1-140273-gaab0bb9fbaa1-dirty #588 > Call Trace: > init_vqs+0x244/0x6c0 drivers/char/virtio_console.c:1878 > virtcons_probe+0x1a3/0x5b0 drivers/char/virtio_console.c:2042 > virtio_dev_probe+0x2b9/0x500 drivers/virtio/virtio.c:263 > call_driver_probe drivers/base/dd.c:515 > really_probe+0x1c9/0x5b0 drivers/base/dd.c:601 > really_probe_debug drivers/base/dd.c:694 > __driver_probe_device+0x10d/0x1f0 drivers/base/dd.c:754 > driver_probe_device+0x68/0x150 drivers/base/dd.c:786 > __driver_attach+0xca/0x200 drivers/base/dd.c:1145 > bus_for_each_dev+0x108/0x190 drivers/base/bus.c:301 > driver_attach+0x30/0x40 drivers/base/dd.c:1162 > bus_add_driver+0x325/0x3c0 drivers/base/bus.c:618 > driver_register+0xf3/0x1d0 drivers/base/driver.c:171 > ... > > Add a suitable sanity check. > > Signed-off-by: Andi Kleen > Signed-off-by: Alexander Shishkin > Cc: Amit Shah > Cc: Arnd Bergmann > Cc: Greg Kroah-Hartman > --- > drivers/char/virtio_console.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 6a821118d553..f4fd5fe7cd3a 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1843,6 +1843,9 @@ static int init_vqs(struct ports_device *portdev) > int err; > > nr_ports = portdev->max_nr_ports; > + if (use_multiport(portdev) && nr_ports < 1) > + return -EINVAL; > + > nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2; > > vqs = kmalloc_array(nr_queues, sizeof(struct virtqueue *), GFP_KERNEL); > -- > 2.39.0 > Why did I only get a small subset of these patches? And why is the whole thread not on lore.kernel.org? And the term "hardening" is marketing fluff. Just say, "properly parse input" or something like that, as what you are doing is fixing assumptions about the data here, not causing anything to be more (or less) secure. But, this still feels wrong. Why is this happening here, in init_vqs() and not in the calling function that already did a bunch of validation of the ports and the like? Are those checks not enough? if not, fix it there, don't spread it out all over the place... thanks, greg k-h