From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-io1-f46.google.com (mail-io1-f46.google.com [209.85.166.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DFF7E20C02D for ; Thu, 23 Jan 2025 15:11:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.166.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737645099; cv=none; b=TrgxpyJRBfYlGUVIfvgg8Z769Z6NluQk9YQ/wEXSjoW9VrAdebDCZLSXbDFlFbM/9pT9H/cytFdrolIxkcgVH73Iy45QdaqZ5a9LZzZeF+VecErqWaqbJcYiXTnatZG5HX1m+Sf/+vO9Hunwxa1i9mF2DkS6ud6pH2HPJmxMqis= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737645099; c=relaxed/simple; bh=UVZ6NArjF1ZcMZyxotnmH4G0QHWDEIgCrE9ut+q8Xc4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=CZLi13nB5VmnZ99J4s03rQGGOkLK5YYPMXKkvR5WBDZlkX6a8y5QF8G3VGEzo/hasLTkcisxvsqtJETJkerFG2d7pGT33J0Vu/XKxtSokoGDzDARLgg4DIRbo0phQgAh15q/g9/jCxMZma07CQIs8kL5ruTTsNpi81hTLqhVkU0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk; spf=pass smtp.mailfrom=kernel.dk; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b=ed59xVzn; arc=none smtp.client-ip=209.85.166.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.dk Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=kernel.dk Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel-dk.20230601.gappssmtp.com header.i=@kernel-dk.20230601.gappssmtp.com header.b="ed59xVzn" Received: by mail-io1-f46.google.com with SMTP id ca18e2360f4ac-844ee15d6f4so76495939f.1 for ; Thu, 23 Jan 2025 07:11:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20230601.gappssmtp.com; s=20230601; t=1737645097; x=1738249897; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=wm83wscrQUkd5TN8872EIFISEWysD3t/grREQ5ujlzc=; b=ed59xVznQlMsauQdnQdYChqnqKYo09oFJPuuMKh7ZyNuxc+9M9XPvdl83c0LhTeEXO Ha/qP4+tcdbcz53UCFyI0RflS/b1V5xVxFb2waHLeAuw/4fLHxHjY9aLHAdxcop+mYzn rRCgcc3IKmTda4CytsqHZlpkMCllmG70R8Y3/Nivy4RRRBiQ77Cy3JNNTMqjT57cv3XV hw1I983hXWxuRSpl1qC4ve2mK+1/mWw7to+J7hB1XHhKWS+rYOeBUmi+0vZ6gzmAShSn eRm/UrxsbQI/6B9WTe1W+fSmBY9wwxIb1axRLjp5It91PwNF1t3hkH4CxpJdu+c/s8Br F16A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737645097; x=1738249897; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wm83wscrQUkd5TN8872EIFISEWysD3t/grREQ5ujlzc=; b=iNMn3bs/56vxUL8LD0LYPHBhUaHghgWLT4d4pvv6mC5McivP7SsD9l7dR++A8K9lBU AGZiZ6n3sakzaixXNXjBElL2IQvclxT6hOXPKQ7fjiK5n873VVZWKtR7/hXX2z12SZY1 nRxdPMZ11Aist+fpjW+izwNPO2Z53zVczdYCY+/ye+/NDb4kxCnB7v2HTrsCNYcrT52T uYjtebJqIahs0iVLWOBl7MsnOSoRCoqQh0UNcIrGP4gUJc08tsjJCRD116ye66c0ycr3 zCTMExYWzphYC6q8R4YUQRic+h2PbITAajuyD9B3VbEU+0OkbQ6MdQGnVn+KKqFPCqMe rzCg== X-Forwarded-Encrypted: i=1; AJvYcCW+mbd7ff+ChjZmPa+y4gbu4QJVrIWNpUh2hT73BAZzSrzks1DmDm4+o34lN2i88vxVvVE8AfO1kuqQIkY=@vger.kernel.org X-Gm-Message-State: AOJu0YwFU/5Sw/tV/YerhIekWZgjXLbJlgGVSHwghj/oU/8UxrzpMT0+ OGE0h2zMR6TAxJEwGvrsk4v7J5dXKyGulOTdqsRr6MXyBq5HD0NghtNGXfnlkLA= X-Gm-Gg: ASbGnctrR2zxe1CMeb5Tf+BR3wCWBwnMQdmIzeb9YvPWRB+PCyHNUVtPuF4/KNyXPgb XEi7m+PqguIrMo1DeNasx5r7bKgR2Wafs/SWYsfVo2lzo+zgVPeOXE9Av406J3EKO22mbN8YCWX 9GfzKmnK7fZNHOpisuqPV6xWYwm3gthubePWXPyhAfPRaH5C4miz1xzJ5q8W6yOUyHPOUwlDdT4 GeLHhKYcAi5Waj+ejxBSAb38846EmNpJzyZyEeBBjVzZM8xMYor8b+/dwrinjFvK3gKt87dQY47 X-Google-Smtp-Source: AGHT+IGe+UG483tiaLa5Jk8Cj7iFRNaZ4YZSe8w/dn+nAvwTG5bujucefh9nlhgP2jHIWeWkQZCw2g== X-Received: by 2002:a92:cda1:0:b0:3a7:955e:1cc5 with SMTP id e9e14a558f8ab-3cf743b9350mr238785155ab.1.1737645096856; Thu, 23 Jan 2025 07:11:36 -0800 (PST) Received: from [192.168.1.116] ([96.43.243.2]) by smtp.gmail.com with ESMTPSA id e9e14a558f8ab-3cf71aae907sm42399885ab.35.2025.01.23.07.11.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Jan 2025 07:11:36 -0800 (PST) Message-ID: <9a4261bd-7c8c-4413-b9ac-ae1aeead1060@kernel.dk> Date: Thu, 23 Jan 2025 08:11:35 -0700 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] io_uring/uring_cmd: add missing READ_ONCE() on shared memory read To: Jann Horn Cc: Pavel Begunkov , io-uring@vger.kernel.org, linux-kernel@vger.kernel.org References: <20250121-uring-sockcmd-fix-v1-1-add742802a29@google.com> <173757472950.267317.14676213787840454554.b4-ty@kernel.dk> <4bf7e5d1-4e66-496d-a503-5dc349efe398@kernel.dk> Content-Language: en-US From: Jens Axboe In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 1/23/25 7:44 AM, Jann Horn wrote: > On Thu, Jan 23, 2025 at 1:18?AM Jens Axboe wrote: >> On 1/22/25 12:38 PM, Jens Axboe wrote: >>> >>> On Tue, 21 Jan 2025 17:09:59 +0100, Jann Horn wrote: >>>> cmd->sqe seems to point to shared memory here; so values should only be >>>> read from it with READ_ONCE(). To ensure that the compiler won't generate >>>> code that assumes the value in memory will stay constant, add a >>>> READ_ONCE(). >>>> The callees io_uring_cmd_getsockopt() and io_uring_cmd_setsockopt() already >>>> do this correctly. >>>> >>>> [...] >>> >>> Applied, thanks! >>> >>> [1/1] io_uring/uring_cmd: add missing READ_ONCE() on shared memory read >>> commit: 0963dba3dc006b454c54fd019bbbdb931e7a7c70 >> >> I took a closer look and this isn't necessary. Either ->sqe is a full >> copy at this point. Should probably be renamed as such... If we want to >> make this clearer, then we should do: > > Are you sure? On mainline (at commit 21266b8df522), I applied the > attached diff that basically adds some printf debugging and adds this > in io_uring_cmd_sock(): Yeah you are right, braino on my part. If we don't go async, it's not copied. The changed fix is still better, but I'll reword the commit message to be more accurate. -- Jens Axboe