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 X-Spam-Level: X-Spam-Status: No, score=-16.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C401C433ED for ; Thu, 20 May 2021 05:43:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4E4EB610A2 for ; Thu, 20 May 2021 05:43:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230381AbhETFpA (ORCPT ); Thu, 20 May 2021 01:45:00 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:48271 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230315AbhETFoz (ORCPT ); Thu, 20 May 2021 01:44:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1621489412; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GjW4Njna3bnKmMjSqk9By1QrG7BREpE+oMFazMCyiuc=; b=DFKJk+yjl5Dj1vVDgxjUjZLYViMF2qhIqkDvlGtqWame69TX01GkBGvlokrAKgoZ724KNc 7wyM0vok8U+0ShkuIp3WAQKV77p9VgMs9oSjUjM9o1wrQW3X0WvBuYG0FzoKm50UsBXlKd uVn4VNwUdkUdXOYfRRWPlFt/0gJJZYU= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-203-5fG6iN4tM_CisFrBsRSGGA-1; Thu, 20 May 2021 01:43:31 -0400 X-MC-Unique: 5fG6iN4tM_CisFrBsRSGGA-1 Received: by mail-wm1-f71.google.com with SMTP id l15-20020a1ced0f0000b029017140e7436dso1027520wmh.2 for ; Wed, 19 May 2021 22:43:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=GjW4Njna3bnKmMjSqk9By1QrG7BREpE+oMFazMCyiuc=; b=D+C1Gu5jRWqG3bVmBzQuDBIuFP9OeC55Puq3A4o9VxltdX32k1GwMvAVz/GBGnUqX/ y9aF6FolbhACLUX3rQqamcfMtNb9l2FQaQl7b8okdKVAMZlacKO4YjzMhXyV1vHfJvqX nM0g8r0Qpyc2cHQbWYHwTO/9oykPfWf0mLMYwIs4/RzoQVq9XVKPSkjHhowmtba8J8Pk y2Vyatg/YXkkprnyOmMqxChfkGRrBaShI0ww0To5ud4ijq64CZwdrEO3vByj2un7lbzI w8VZpU7gPTwS9mxgm+ZzBzbboSg5wf9opV2aPl6LKoYAtO+Xg+cxiDZ2RNtlaMX+xnQj KCng== X-Gm-Message-State: AOAM531yMbF3j6g42Pn14jnzA9xWQHPkqk5AHGhjh5EJkyYBz4PH+yvt 0ACc/CSxWC3NAzSfJmoG7DDNOGdLHQxk7GImYXD1vc+ayxW0SyHG/jrr+6sajj88NJRP8B58E6p PEeK2SGI8fyvJ20ydRWLvYYcf X-Received: by 2002:a5d:524b:: with SMTP id k11mr2308301wrc.292.1621489410041; Wed, 19 May 2021 22:43:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwPmuJaJcBIzoz51cJHFKBEGSMwRicV/EJnxc2pubykB7VVTdZU5S/RsC7BPoTegDy8gNdFLQ== X-Received: by 2002:a5d:524b:: with SMTP id k11mr2308281wrc.292.1621489409886; Wed, 19 May 2021 22:43:29 -0700 (PDT) Received: from redhat.com (bzq-79-181-160-222.red.bezeqint.net. [79.181.160.222]) by smtp.gmail.com with ESMTPSA id t16sm1717230wrb.66.2021.05.19.22.43.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 May 2021 22:43:29 -0700 (PDT) Date: Thu, 20 May 2021 01:43:25 -0400 From: "Michael S. Tsirkin" To: Yongji Xie Cc: Dan Carpenter , Jason Wang , Stefan Hajnoczi , Stefano Garzarella , Parav Pandit , Christoph Hellwig , Christian Brauner , Randy Dunlap , Matthew Wilcox , viro@zeniv.linux.org.uk, Jens Axboe , bcrl@kvack.org, Jonathan Corbet , Mika =?iso-8859-1?Q?Penttil=E4?= , joro@8bytes.org, virtualization , netdev@vger.kernel.org, kvm , linux-fsdevel@vger.kernel.org, iommu@lists.linux-foundation.org, linux-kernel Subject: Re: Re: [PATCH v7 04/12] virtio-blk: Add validation for block size in config space Message-ID: <20210520013921-mutt-send-email-mst@kernel.org> References: <20210517095513.850-1-xieyongji@bytedance.com> <20210517095513.850-5-xieyongji@bytedance.com> <20210519144206.GF32682@kadam> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 20, 2021 at 01:25:16PM +0800, Yongji Xie wrote: > On Wed, May 19, 2021 at 10:42 PM Dan Carpenter wrote: > > > > On Wed, May 19, 2021 at 09:39:20PM +0800, Yongji Xie wrote: > > > On Mon, May 17, 2021 at 5:56 PM Xie Yongji wrote: > > > > > > > > This ensures that we will not use an invalid block size > > > > in config space (might come from an untrusted device). > > > > I looked at if I should add this as an untrusted function so that Smatch > > could find these sorts of bugs but this is reading data from the host so > > there has to be some level of trust... > > > > It would be great if Smatch could detect this case if possible. The > data might be trusted in traditional VM cases. But now the data can be > read from a userspace daemon when VDUSE is enabled. > > > I should add some more untrusted data kvm functions to Smatch. Right > > now I only have kvm_register_read() and I've added kvm_read_guest_virt() > > just now. > > > > > > > > > > Signed-off-by: Xie Yongji > > > > --- > > > > drivers/block/virtio_blk.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > > > > index ebb4d3fe803f..c848aa36d49b 100644 > > > > --- a/drivers/block/virtio_blk.c > > > > +++ b/drivers/block/virtio_blk.c > > > > @@ -826,7 +826,7 @@ static int virtblk_probe(struct virtio_device *vdev) > > > > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE, > > > > struct virtio_blk_config, blk_size, > > > > &blk_size); > > > > - if (!err) > > > > + if (!err && blk_size > 0 && blk_size <= max_size) > > > > > > The check here is incorrect. I will use PAGE_SIZE as the maximum > > > boundary in the new version. > > > > What does this bug look like to the user? > > The kernel will panic if the block size is larger than PAGE_SIZE. Kernel panic at this point is par for the course IMHO. Let's focus on eliminating data corruption for starters. > > A minimum block size of 1 seems pretty crazy. Surely the minimum should be > higher? > > > > Yes, 512 is better here. > > Thanks, > Yongji