From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f180.google.com (mail-oi1-f180.google.com [209.85.167.180]) (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 2E1D82F851 for ; Mon, 23 Mar 2026 01:04:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774227879; cv=none; b=p4Q8kFXsavRyFdoqDmmNrkXpI0uVNvR5bps//kbLTi2DobZNifFxBdKrgSs0UN9d5GKTwtRQBdVIz4YjetK+BMd4dt571E1ZvzOBSz2Nebbj4PS4aYdaQJL3tB0+RROPyhCLTxtDfCqSiDGBVU63dgXNH2bEy7wrBzbq9LBKTYo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774227879; c=relaxed/simple; bh=1858aEuXQCDBFZbM6Amq89zSY2nlYPEvVwLAwgsvue0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=J52yU7cWL66keVGkitccvcg3KyKMmdqqPwSEjos5bo+iqPl7uYRoVojySnoRxTMxaga79j6mDECPFWddCdqNAvebn906fZC9fFnJqXSbFyxuNRuO/g5sHy90kgLoeqfnA7y+VI+T0FqaEK4svdihIvI87oJgu1cmwS/a6GEQ8Ag= 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=G9Kyc+v2; arc=none smtp.client-ip=209.85.167.180 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="G9Kyc+v2" Received: by mail-oi1-f180.google.com with SMTP id 5614622812f47-46701f2077cso3494643b6e.0 for ; Sun, 22 Mar 2026 18:04:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20230601.gappssmtp.com; s=20230601; t=1774227877; x=1774832677; 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=w1B1Svo5mktAT8Mqk4J/FNWcbOe8c+HvGIGaqad1TNU=; b=G9Kyc+v2awZBS+/v26IKHJ21bcY4Pc7iQIY4ZLvnZQSZDDO+yi5WyUqyFsOTSOrYsy +Vvh/9qDsOEOX5gF/Q9mpwN0V4O7unQq0DzdjmFcixVbiX24vXAcRtZVWqN1HlC8U8r0 rsKMogLqSigh3GmEpQQwDJnvX7p9lCso1ifUbJl2S7ckcu6Ogxg1c9b3vg+gh4C9W3qm Ev9GUKAmMcKfe5KvC03vwY8Y5uZBb3LRm4LJ+dM36X1OX0WzVpioDmUc3RdV6sB8K+qp xKO0yZ9saxndv4tnjalLI8YiFUbymfVfZxpkd9FAPcamxy0s385VdfldmWDXMwC+4KSC FceA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774227877; x=1774832677; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=w1B1Svo5mktAT8Mqk4J/FNWcbOe8c+HvGIGaqad1TNU=; b=KvfyrDpg7CgfhVYmYDROzyagLoZbyjLtCIk5ARnhBZgFeopU9SWCoAlc/0NQF9/7Ab G6be34GN/jqA1gQZ4AeAogrhKoj/zhEojXdf63MeLHzHcEUpoU0KJXfmtUJ3Q+yIO/pa GbAIN5q0++b9mgLd6h0DmHNpsNRTfNF7radFZv58t5UFEj6F047A9dyOtmDlOCRe/b4M wFr1sFUdrTQqEW+v2SaKAKvrWI+CV6Q+TAhRj7lpMJlnQrAEeM2Ggseyylrdb7pBhIct kLAD+sZgUsds1+qwt4Cr1/bnbf5I8jL3NRAP6LEUhbZi/P9dhJIa0Q4Vc0m9ZG54jDmF Evuw== X-Forwarded-Encrypted: i=1; AJvYcCWxB140KlHssryWS62jlhWjFwApaSShhpUWovMy6cwtw/XQ7elaY6cPBoipbCdXWgSjR8VrJNKV5S28jqA=@vger.kernel.org X-Gm-Message-State: AOJu0Yxkkf2M1BZgOWzik8lPWzbJyykBI6vYixaU1coR0P6A98WGZjdi zbm6O0k82vcA5lYGuNHsMrZtsJUCPhTpp0bD2lOU+uqUWOY50afhcTfWgy+xbB3Ex3Q= X-Gm-Gg: ATEYQzzehHAHKXA3yxR6qABPt9wRlRAvrZANCgcs0J7JPSubzEMENP2uGqUykvGUVOs TE6J9apNBoqV68vU6DHU789i/ztoDYUzC/22txrC5dkiSee4XAxA4be5ojAcSoF/I+d7HP/hKLd 97Hx3+qqgZxk/A4N20VxovfL+dCiKEB3TVWE//NAu+baEurweVWLedpAhYCyG45J2Q0GtOk1eoF 5TG5CUxgMmD8Y8AJI2F69Fm1tDOneCkdjyon0tNF4DYxPzZ5en+RN7TDkO0FPLLO9rxRcbYBf8S bwhgiw/s+Kl59E1KiWVCzKdxFhFzo2tj4R6940dJoVMdJo+AXGtPxhsWnLhnu2YpkC897lbzUI7 xWs87AuffabexXDwUi0CYjKkFburwPLX4Qik2dN6ZFMnNT3Ku73ng/Th60qkLMpomUoW3qMaFwj ZKbTikc+7GxQwItyhAZNssE+Nx+6P8AfU6LkuhA7tqB2BjDK0d1gyztHWGs16azjNpkoSacxcT5 P5Y2aMRIA== X-Received: by 2002:a05:6808:1926:b0:467:fb12:c9f6 with SMTP id 5614622812f47-467fb12caf7mr4373668b6e.14.1774227877031; Sun, 22 Mar 2026 18:04:37 -0700 (PDT) Received: from [192.168.1.150] ([198.8.77.157]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-41c14d63e29sm8851293fac.12.2026.03.22.18.04.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 22 Mar 2026 18:04:36 -0700 (PDT) Message-ID: <6fab0722-cf39-4081-a337-0be8e6ffc26d@kernel.dk> Date: Sun, 22 Mar 2026 19:04:35 -0600 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] bsg: initialize request and reply payloads in bsg_prepare_job To: jonghwi.rha@samsung.com, Hannes Reinecke Cc: "linux-block@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "hch@lst.de" , =?UTF-8?B?6rmA7KCV7YOc?= , =?UTF-8?B?7KCV7Zic7Jew?= References: <20260318102030epcms2p7b2daaab73032a6a26eca9c8307a7322e@epcms2p7> Content-Language: en-US From: Jens Axboe In-Reply-To: <20260318102030epcms2p7b2daaab73032a6a26eca9c8307a7322e@epcms2p7> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 3/18/26 4:20 AM, ??? wrote: > On 2/6/26 13:58 PM, ??? wrote: >> On 2/6/26 00:45, Hannes Reinecke wrote: >>> On 2/5/26 14:42, Jens Axboe wrote: >>>> On 2/4/26 10:32 PM, ??? wrote: >>>>> bsg: initialize request and reply payloads in bsg_prepare_job >>>>> >>>>> struct bsg_job payloads contain fields that are only populated by >>>>> certain commands, such as sg_list pointers. >>>>> >>>>> Because struct bsg_job is allocated with kmalloc(), memory may be >>>>> reused across requests. If a command does not populate all payload >>>>> fields, stale state from a previous job may remain and later be >>>>> misinterpreted during cleanup, potentially leading to use-after-free >>>>> or double-free issues. >>>>> >>>>> Initialize both request and reply payloads at the beginning of job >>>>> preparation to ensure a clean state for all commands. >>>>> >>>>> Signed-off-by: Jonghwi Rha >>>>> >>>>> diff --git a/block/bsg-lib.c b/block/bsg-lib.c >>>>> index 32da4a4429ce..0fbf8e311c03 100644 >>>>> --- a/block/bsg-lib.c >>>>> +++ b/block/bsg-lib.c >>>>> @@ -234,6 +234,12 @@ static bool bsg_prepare_job(struct device *dev, struct request *req) >>>>> struct bsg_job *job = blk_mq_rq_to_pdu(req); >>>>> int ret; >>>>> >>>>> + /* Clear stale SG state since bsg_job is reused as a request PDU */ >>>>> + job->request_payload.sg_list = NULL; >>>>> + job->request_payload.sg_cnt = 0; >>>>> + job->reply_payload.sg_list = NULL; >>>>> + job->reply_payload.sg_cnt = 0; >>>>> + >>>>> job->timeout = req->timeout; >>>>> >>>>> if (req->bio) { >>>> >>>> The patch is white-space damaged, tabs are spaces. But I can fix that >>>> up. Do we just want to do a memset(job, 0, sizeof(*job)) here to avoid >>>> any oddities like this in the future? >>>> >>> >>> That might indeed be better. >> >> The suggested method impairs normal operation. If bsg_prepare_job performs >> a zero?memset for the job structure, all request?related information set on >> the driver side before the call will be lost. Therefore, if it runs as is, >> it will go to ufs_bsg_request and cause a null?pointer access. >> >> Currently, the original patch has no functional impact. >> >> The blank problem seems to be due to a mistake I made while copying and pasting >> the patch. I am reattaching the patch below. If needed, I can attach the patch >> and resend the new email. >> >> >> [PATCH] bsg: initialize request and reply payloads in bsg_prepare_job >> >> struct bsg_job payloads contain fields that are only populated by >> certain commands, such as sg_list pointers. >> >> Because struct bsg_job is allocated with kmalloc(), memory may be >> reused across requests. If a command does not populate all payload >> fields, stale state from a previous job may remain and later be >> misinterpreted during cleanup, potentially leading to use-after-free >> or double-free issues. >> >> Initialize both request and reply payloads at the beginning of job >> preparation to ensure a clean state for all commands. >> >> Signed-off-by: Jonghwi Rha >> --- >> block/bsg-lib.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/block/bsg-lib.c b/block/bsg-lib.c >> index 32da4a4429ce..0fbf8e311c03 100644 >> --- a/block/bsg-lib.c >> +++ b/block/bsg-lib.c >> @@ -234,6 +234,12 @@ static bool bsg_prepare_job(struct device *dev, struct request *req) >> struct bsg_job *job = blk_mq_rq_to_pdu(req); >> int ret; >> >> + /* Clear stale SG state since bsg_job is reused as a request PDU */ >> + job->request_payload.sg_list = NULL; >> + job->request_payload.sg_cnt = 0; >> + job->reply_payload.sg_list = NULL; >> + job->reply_payload.sg_cnt = 0; >> + >> job->timeout = req->timeout; >> >> if (req->bio) { >> -- > >> Regards, >> Jonghwi, > > -- > > Since there was no reply, I am resending the email as a reminder. > First, I have confirmed in my environment that, as you suggested, > memset?as 0 for all 'job' struct elements eventually results an error. > The reason is, as I mentioned above, that the request/reply gets lost > before re-using. > > Also, since other elements in the structure are reused, so they are > not relevant to the current issue. > > If the code execution point is not ideal, there is also the option of > zeroising after freeing the memory allocation. Just send it out as a proper patch and we can take a look at it again. -- Jens Axboe