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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7CA10C433EF for ; Fri, 25 Mar 2022 07:08:01 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3B99584164; Fri, 25 Mar 2022 08:07:59 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="bFXkl3OK"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 3F16584167; Fri, 25 Mar 2022 08:07:58 +0100 (CET) Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id A64CF83994 for ; Fri, 25 Mar 2022 08:07:55 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ascull@google.com Received: by mail-qv1-xf31.google.com with SMTP id r1so5546953qvr.12 for ; Fri, 25 Mar 2022 00:07:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=og53NEgE1Q0/VD8Vx8wL55RIocZpcGM4ciSQ2UQGO7E=; b=bFXkl3OKY4F5om5NtIskxPs2aG8jkp8GldYS+zM8NQ3VZJR4FoQx1pI2daba8h8+dU /1agnNAMpnYDqcfIwwklzjf7WmyyHPnDYx9VVU2Zjy47kNDTX0MdnGtX7TyOIoICzo6r Y3IOEr5w/yqXP0n3BVCArRtMvjeUeJ19drlx7IrWUvDnK3qeORa+1FndIiVoDvnX7fT4 i9LM6M4QB+g5WRswGCf5/Epi4FWjpxqeLNgabsVYSt/O5KIfRkFcOp2l2lVw0PRQrweR r2YZnaQ4keA6oCjUXI7iq8kmLUx0e8Tn9ht5vDq7hgDAubXhncfrEBr3QdHvGHLNIu9y oEoQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=og53NEgE1Q0/VD8Vx8wL55RIocZpcGM4ciSQ2UQGO7E=; b=33WTrYmxmNx7m2AhF/B0aF+hrfTzzZTlUJXEpKJ3D7XEU6kB+267T2hlMppSCkZCbS tXcVqT8U0iksZTzKfsDBue/zsyT+v1k5IwBmGacK0+rfZUh0gX84xekUz8UW1mHqJUEk yY+0/WU6GFA7O9KJeX78QdVuhYja69PxhXtBRYdyGmQvd9/kBAS6c/swBKqK+JAAvbj2 +4nivWBB2i2h/R7JpX3aZ3D/QTUHuoML6968uZNW9oW5ZeGODRg6E0QFsWtpPyT+BbR2 sg0SZ02fm/nmRj59FdPqJlYcTNCHTrshzaTM/CTGq+d9MWWKwxcQ8/X9ee2ZsLe+tg79 4rKw== X-Gm-Message-State: AOAM533FhQ3yz+ZdjmIp6haIBbLPrXpUE8r23AiJ7xQxQfMEg6TZTSAZ wH5fEtu+ZaxBEO84ldWPpv16IXTUOo6K8EC/ggXkOQ== X-Google-Smtp-Source: ABdhPJxomsmoVINSzfxWfn/4CXWMnl2kwGsWW8IU1QidoQzBEJ0SOVcUsCWWFo+7GhXsmnbpmQERcLLbJq4TIjFdarI= X-Received: by 2002:a05:6214:4119:b0:441:9eb:d4ba with SMTP id kc25-20020a056214411900b0044109ebd4bamr7802291qvb.54.1648192074438; Fri, 25 Mar 2022 00:07:54 -0700 (PDT) MIME-Version: 1.0 References: <20220320114118.2237795-1-ascull@google.com> <20220320114118.2237795-8-ascull@google.com> In-Reply-To: From: Andrew Scull Date: Fri, 25 Mar 2022 07:07:43 +0000 Message-ID: Subject: Re: [PATCH 07/11] virtio: pci: Check virtio configs are mapped To: Bin Meng Cc: U-Boot Mailing List , Simon Glass , Alistair Delva , Keir Fraser , =?UTF-8?Q?Pierre=2DCl=C3=A9ment_Tosi?= Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean On Fri, 25 Mar 2022 at 04:38, Bin Meng wrote: > > On Sun, Mar 20, 2022 at 7:42 PM Andrew Scull wrote: > > > > The calls to `virtio_pci_map_capability()` return NULL on error. If this > > happens, later accesses to the pointers would be unsafe. Avoid this by > > failing if the configs were not successfully mapped. > > > > Signed-off-by: Andrew Scull > > --- > > drivers/virtio/virtio_pci_modern.c | 25 ++++++++++++++++++++----- > > 1 file changed, 20 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > > index 38a0da0a84..2f1a1cedbc 100644 > > --- a/drivers/virtio/virtio_pci_modern.c > > +++ b/drivers/virtio/virtio_pci_modern.c > > @@ -534,7 +534,19 @@ static int virtio_pci_probe(struct udevice *udev) > > return -EINVAL; > > } > > > > + /* Map configuration structures */ > > + priv->common = virtio_pci_map_capability(udev, &common_cap); > > + if (!priv->common) { > > This can't be NULL, as you did not pass a NULL capability pointer > > > + printf("(%s): could not map common config\n", udev->name); > > + return -EINVAL; > > + } > > + > > priv->notify_len = notify_cap.length; > > + priv->notify_base = virtio_pci_map_capability(udev, ¬ify_cap); > > + if (!priv->notify_base) { > > + printf("(%s): could not map notify config\n", udev->name); > > + return -EINVAL; > > + } > > > > /* > > * Device capability is only mandatory for devices that have > > @@ -543,13 +555,16 @@ static int virtio_pci_probe(struct udevice *udev) > > device = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_DEVICE_CFG, > > sizeof(struct virtio_pci_cap), > > &device_cap); > > - if (device) > > + if (device) { > > priv->device_len = device_cap.length; > > + priv->device = virtio_pci_map_capability(udev, &device_cap); > > + if (!priv->device) { > > + printf("(%s): could not map device config\n", > > + udev->name); > > + return -EINVAL; > > + } > > + } > > > > - /* Map configuration structures */ > > - priv->common = virtio_pci_map_capability(udev, &common_cap); > > - priv->notify_base = virtio_pci_map_capability(udev, ¬ify_cap); > > - priv->device = virtio_pci_map_capability(udev, &device_cap); > > debug("(%p): common @ %p, notify base @ %p, device @ %p\n", > > udev, priv->common, priv->notify_base, priv->device); > > > > I don't think adding these checks is necessary. See later patches in the series that validate the address range is within the declared PCI ranges and not an arbitrary range chosen, accidently or maliciously, by the device. If those checks fail, the memory will not have been mapped and the probe should fail. > Regards, > Bin