From patchwork Wed Jun 21 07:21:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 13286784 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7D2DBEB64DD for ; Wed, 21 Jun 2023 07:21:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 09E2910E3CF; Wed, 21 Jun 2023 07:21:23 +0000 (UTC) Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5BCAD10E3CF for ; Wed, 21 Jun 2023 07:21:21 +0000 (UTC) Received: by mail-wm1-x330.google.com with SMTP id 5b1f17b1804b1-3f918922954so32080255e9.2 for ; Wed, 21 Jun 2023 00:21:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1687332078; x=1689924078; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=O8Qxh2dT/vwmYCb8MwuNXPbVaEQPUFjJDHObAtD1Tv0=; b=ZfXGWQRSl4/XvBX3iv+Rh/4qYvqpdCuRzYsg8C2NverEOCaCnpi3ecSOWQ0UoImMUl Gu1kywphP0el0ZT7nUq6CEIBna70laq8q3Ry+fnYkaDngMs7rJstJCWwUdMvbjpDvXH5 e9MnKB0KebdorG5JwPVJXu40zDYi8xtBhdhLNDpCSpG97++FXB5zq9XQ6/RCcRTos4LM ANiy3a0CE1hvSFqDBCvcga0xjEya9vc6XXdSvGKaIz+aZzlX50BhKtZoAbJvMpcYl/2T Ht/Fk2fWo6jIbHV4TFToiz7RE7tKYKkenPYnY+qERubDNhdObSJNsm5gRcIwOC2DsAdR Z80A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687332078; x=1689924078; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=O8Qxh2dT/vwmYCb8MwuNXPbVaEQPUFjJDHObAtD1Tv0=; b=ddttDywnDehlzo+SWCOxeKkkNkp21nvSSEliEXpiuxrXnAn/U4Z3yZu3i3Lv1tyHE2 szxU0Cbf3eJuwPhAaxLvdq7irzC9QLPzxKYmDVavM0siRhL+Ta6NsnEM7XsnExT7P/4I gVxH4X2YVBa/cNhWZ5dbl0Do6QjeisEz9rXOIZw7PDJXpkiD1fHGVdXh2/EVr0lJysvG YOXOckRAEAJ0dE0WtO/5qqKSB0mF0jky1bylTRenDhOTNon08pEJGHmnwg4cKZ1dA+ZN 7/Dckcw0AWcaVGII/r7tE40NnOLXyNEwerVsUeP51vTBISFEpsgVYmAbaQ5tn4JTnTuQ L5dA== X-Gm-Message-State: AC+VfDx09p+2n+YwJyYojkBIP5wu0cTBpYXighAvnut2/LFQyLxGi5zx w5izcs61bcQg47ycDoh8XxE7Dw== X-Google-Smtp-Source: ACHHUZ6XpM6zbQ+qRrWux2KVECElKBl33INbEX4v/GZyD4Exilk6QiXFk7yV4s2Fau3BgEusHee8EA== X-Received: by 2002:a5d:5960:0:b0:30f:c56c:b5b0 with SMTP id e32-20020a5d5960000000b0030fc56cb5b0mr10339083wri.4.1687332078132; Wed, 21 Jun 2023 00:21:18 -0700 (PDT) Received: from localhost ([102.36.222.112]) by smtp.gmail.com with ESMTPSA id d2-20020adff842000000b00312793cc763sm3735043wrq.15.2023.06.21.00.21.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Jun 2023 00:21:16 -0700 (PDT) Date: Wed, 21 Jun 2023 10:21:12 +0300 From: Dan Carpenter To: Jeffrey Hugo Subject: [PATCH 0/5] accel/qaic: Improve bounds checking in encode/decode Message-ID: MIME-Version: 1.0 Content-Disposition: inline X-Mailer: git-send-email haha only kidding X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arm-msm@vger.kernel.org, Oded Gabbay , kernel-janitors@vger.kernel.org, dri-devel@lists.freedesktop.org, Pranjal Ramajor Asha Kanojiya , Carl Vanderlip Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" (I think this is the first cover letter I have ever written). These patches are based on review and not from testing. I found it quite complicated to track the buffer sizes. What happens is the qaic_manage() gets a buffer user_msg->data[] which has user_msg->len bytes. The qaic_manage() calls qaic_manage_msg_xfer() which encodes the user's message. Then we get a response and we decode the response back into user_msg->data[], but we don't check that it is overflowed. We instead copy seem to check against msg_hdr_len (which would prevent a read overflow). At the end user_msg->len gets set to the number of bytes that we copied to the buffer. I'm coming to this code brand new, it's the first time I have seen it. So I don't really understand. There is an element of trust in msg_hdr_len but then at other times we check it for integer overflows which indicates deep distrust. What I'm saying is that there may be more issues in this code. But also that I don't really understand it so please review carefully. The patch that I'm least sure of is 4/5: [PATCH 4/5] accel/qaic: move and expand integer overflow checks for map_user_pages() regards, dan carpenter