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=-5.7 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no 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 0044EC47080 for ; Tue, 1 Jun 2021 20:07:01 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B105660FD8 for ; Tue, 1 Jun 2021 20:07:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B105660FD8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Subject:Cc:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=BuB2NqNATZUqjSQgq5MvdpK4bSoA0uymzK7x10IVrG0=; b=CYM+M4T3J61baf H/Go3ZFvySWwIhYVQecI1gv0/q+USikfiSvcbpx9DTgz68NWNbAZwfujneBhgEzSxAXiFt6fNH661 Wik9jbiXl4M5/pxh4XOtkAZYzR5HSn2gJf845pyzMQrgp9SeXqXiemWq5DBkYwlP4j9qUzLL/92bN mGs424COVysuiIe8yZSsHAZjjCL9nvafQ8lutwi7gw9qkTv36UT14YfR1hs6AaLFDJvY3VMKq3Q+G cuuM/q3m+2IYJvk7+axtXjgCiScjxWvML4d5Ql2Uwcxa8VZZJhDsBaMSsF7lOX9lY6fRkd/GxFOFc f44hFFlxcg5sEwl2xd0w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1loAdn-000gIq-Vb; Tue, 01 Jun 2021 20:05:32 +0000 Received: from mail-pg1-x534.google.com ([2607:f8b0:4864:20::534]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1loAdk-000gFc-4m for linux-arm-kernel@lists.infradead.org; Tue, 01 Jun 2021 20:05:29 +0000 Received: by mail-pg1-x534.google.com with SMTP id t8so251608pgb.9 for ; Tue, 01 Jun 2021 13:05:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:mime-version:content-disposition; bh=unxYYUIJtzBYSX4MANGuQqbleLprmVLiT5PFu/4IgM8=; b=I4Y2WG6Fm0u56jprweWhvuUHM1OeSnYiQyyVI2kWvbTGhM22BtdsOkQBPkjs31huYB Mmzi/aCBh+wLAEaOlC30D+9T8IItdg638U/Poy1jQVG4vIMrqd8BLVayZfriNqFUWcoK KKhO7ietySBL41WRLIdm0WigKvuKY7tnkbb2lafcRFNZbvEQ5r8N3pzoBH/JPoX3Ux1R bpcyWADATVqSHOW07R3fQYLATcL1Tfsfeb4Ry3toczn+qmm4tgYpcLnmw3MtV+oJ2vHI fXqk6QJsgD7g04fmJW9t3TY/4eT4k167TNOJX05Q7kUErUl6ewk72PRaenwmnUS1x6+0 Og5Q== 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:mime-version :content-disposition; bh=unxYYUIJtzBYSX4MANGuQqbleLprmVLiT5PFu/4IgM8=; b=BFITqWXkCYHxKHFwlEkPgKHsc7S7qgF4evHIrWH8NH/E4eILAlGcohdjR0EgSOBORv z/Dz9SCzfoPP7CRyG73820mFLMo/vxbq+vhYmjHsKNgLOtwPblBkcg4tJYYO6mLE8abv VKJLhoJQSkNhv1kfMH2QTK+jCEaQvnyQIE/sSGjwWSeIUBlgfkQYmBsY2W6mafx+6aHB fVtGXolwAFCc2s3JyGRGq+ahPSRjeXxEPQiviwLik6WoHipXxOQvzV5cN4D5h59Yd//Y LJI7mJ0hVeFVRYVzJ+ZxzBUC89pc9mQFbuAa+xfuvucC0xBDTmj4g3HvDHMutVYdV71P xZJw== X-Gm-Message-State: AOAM532GzfvTOdTJ8i+R9StN2K9pa6+CuZiZlANEDhdo4fmgI6vPsm3F wZEx3dDd9QlNVrqXx+2eH6w= X-Google-Smtp-Source: ABdhPJwxegWvJ6SacBtxU7EltgrnbygfeJpMF1B4eYwaVNQ9R7wOaR6qDVUnnGdIsdi4HMskObkOyg== X-Received: by 2002:a63:1559:: with SMTP id 25mr30519910pgv.384.1622577926897; Tue, 01 Jun 2021 13:05:26 -0700 (PDT) Received: from ojas ([2405:204:130c:d068:69c0:20e2:ff2e:7df0]) by smtp.gmail.com with ESMTPSA id z19sm2747045pjq.11.2021.06.01.13.05.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 01 Jun 2021 13:05:26 -0700 (PDT) Date: Wed, 2 Jun 2021 01:35:13 +0530 From: Ojaswin Mujoo To: nsaenz@kernel.org Cc: gregkh@linuxfoundation.org, arnd@arndb.de, dan.carpenter@oracle.com, phil@raspberrypi.com, bcm-kernel-feedback-list@broadcom.com, linux-arm-kernel@lists.infradead.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: staging: vc04_services: Need suggestions on trying to fix sparse warning in vchiq_arm.c Message-ID: <20210601200513.GA10204@ojas> MIME-Version: 1.0 Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210601_130528_250577_BFEB5333 X-CRM114-Status: GOOD ( 20.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hello, I was trying to address the following TODO item in "drivers/staging/vc04_services/interface/TODO" and would love some comments and suggestions on this: > 14) Clean up Sparse warnings from __user annotations. See > vchiq_irq_queue_bulk_tx_rx(). Ensure that the address of "&waiter->bulk_waiter" > is never disclosed to userspace. More specifically, I was looking at ways to fix the following sparse warning: drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1001:26: warning: incorrect type in assignment (different address spaces) expected void *[assigned] userdata got void [noderef] __user *userdata >From my understanding, the issue seems to be that the (void *)userdata local variable in vchiq_irq_queue_bulk_tx_rx() can be assigned a (void __user *) or a (struct bulk_waiter *). This makes the assignment tricky since it can either be a userspace pointer or a kernel pointer. Right now, we are just ignoring the sparse warning which is essentially resulting in the __user attribute being lost when we assign args->userdata to userdata. This can be dangerous as it might result in illegal access of userspace memory, without any sparse warning, down the line. Further, this issue seems to boil down to the fact that the (void *)userdata field in struct vchiq_bulk (in vc04_services/interface/vchiq_arm/vchiq_core.h) can, again, either be a (stuct bulk_waiter *) or a (void __user *). To fix this, I was playing with the idea of modifying this userdata field of struct vchiq_bulk to be something like the following: diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/ drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -227,10 +227,16 @@ enum vchiq_bulk_dir { typedef void (*vchiq_userdata_term)(void *userdata); +struct vchiq_userdata_or_waiter { + void __user *userdata; + struct bulk_waiter *bulk_waiter; + +}; + struct vchiq_bulk { short mode; short dir; - void *userdata; + struct vchiq_userdata_or_waiter *userdata_or_waiter; dma_addr_t data; int size; void *remote_data; I was then planning on modifying all the code that works with vchiq_bulk->userdata accordingly. I believe this can help us overcome the sparse warnings, as well as preserve the __user attribute in cases when the userspace pointer does get assigned here. However, since I'm not very familiar with the codebase, I just wanted to confirm if this is an acceptable workaround and to make sure I'm not breaking anything or overlooked anything here. I noticed that we also want to make sure that bulk_waiter's address is not exposed to userspace. Will it be possible to provide some pointers on how/where this might happen, so I can see if I can try to extend this patch to avoid that. I would love to hear you suggestions and thoughts on this. PS: I'm new here so please do correct me incase I missed anything. Regards, Ojaswin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel