From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ns2.wdyn.eu (ns2.wdyn.eu [5.252.227.236]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 890CD548F1; Thu, 4 Apr 2024 06:55:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=5.252.227.236 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712213747; cv=none; b=WVe6mGnKWeMBbD7hLvgdRrhCaPyt/je20uD8ywI0Tb18LpamjGTx3r/HEY/ODaxb42fJWiQ9QATcx1ndUHcoD65rznhZXEVt0e7D+sIswZQGUi6va6KJwF438vXvK44KPqxsxiPDR5OqeHoGCGyNtJfjknlAxb/yokFsBvqeMQU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712213747; c=relaxed/simple; bh=/TkSoJVhwN0sFa9XmDOyKapqxmji/XCgpaNo1YUh5MY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=mCJvKHxDN1aHR6zo+atl4zoelFhmX+AoCPkzobgkIv1tjz4iGrzzVk7Suy8yNkMcBnoiH/MNtgMnF+CaOyx2vrKoTzjVgXAJYRrDyDvTPdWQdF+0DdbgWwko0w/NhDtKuZypmWrq774H5WgRbSfP1O8mUDVbvD/PE0lsihrpTBA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=wetzel-home.de; spf=pass smtp.mailfrom=wetzel-home.de; dkim=pass (1024-bit key) header.d=wetzel-home.de header.i=@wetzel-home.de header.b=mhkEJjAi; arc=none smtp.client-ip=5.252.227.236 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=wetzel-home.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=wetzel-home.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=wetzel-home.de header.i=@wetzel-home.de header.b="mhkEJjAi" Message-ID: <5cbb1668-1640-4a5d-b6a8-ed0e5840b9ee@wetzel-home.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=wetzel-home.de; s=wetzel-home; t=1712213733; bh=/TkSoJVhwN0sFa9XmDOyKapqxmji/XCgpaNo1YUh5MY=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=mhkEJjAi8tugEdCRcm1yf39ytSuvVtyOezlR1jntHq0B2m3i9GCfRSh9RxMW7cYJd 3tvtox0qhFFoCV6B0QP4LrVGHYgQfF/sD3Qt+EW2KbhI6VvI8pBhvxgufc7BKLK94h O+vHqg0FV/kBEnDV7hbd4ggD2HVFOYxuq6NVZATY= Date: Thu, 4 Apr 2024 08:55:30 +0200 Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] scsi: sg: Avoid race in error handling & drop bogus warn To: Bart Van Assche , dgilbert@interlog.com Cc: gregkh@linuxfoundation.org, sachinp@linux.ibm.com, linux-scsi@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, martin.petersen@oracle.com, stable@vger.kernel.org References: <81266270-42F4-48F9-9139-8F0C3F0A6553@linux.ibm.com> <20240401191038.18359-1-Alexander@wetzel-home.de> Content-Language: en-US, de-DE From: Alexander Wetzel In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 04.04.24 01:24, Bart Van Assche wrote: > On 4/1/24 12:10 PM, Alexander Wetzel wrote: >> @@ -301,11 +302,12 @@ sg_open(struct inode *inode, struct file *filp) >>       /* This driver's module count bumped by fops_get in */ >>       /* Prevent the device driver from vanishing while we sleep */ >> -    retval = scsi_device_get(sdp->device); >> +    device = sdp->device; >> +    retval = scsi_device_get(device); >>       if (retval) >>           goto sg_put; > > Are all the sdp->device -> device changes essential? Isn't there a > preference to minimize patches that will end up in the stable trees? > Only the very last change is essential: - scsi_device_put(sdp->device); - goto sg_put; + kref_put(&sdp->d_ref, sg_device_destroy); + scsi_device_put(device); + return retval; Not using a (required) local variable and de-referencing it again and looks strange for anyone reading the code. While the additional lines in the patch are trivial to review... Alexander 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 Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 D948BCD1292 for ; Thu, 4 Apr 2024 06:56:34 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; secure) header.d=wetzel-home.de header.i=@wetzel-home.de header.a=rsa-sha256 header.s=wetzel-home header.b=mhkEJjAi; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4V9C8K24lyz3f0P for ; Thu, 4 Apr 2024 17:56:33 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=wetzel-home.de header.i=@wetzel-home.de header.a=rsa-sha256 header.s=wetzel-home header.b=mhkEJjAi; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=wetzel-home.de (client-ip=2a03:4000:40:5b2::1; helo=ns2.wdyn.eu; envelope-from=alexander@wetzel-home.de; receiver=lists.ozlabs.org) Received: from ns2.wdyn.eu (ns2.wdyn.eu [IPv6:2a03:4000:40:5b2::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4V9C7S00zZz3cWB for ; Thu, 4 Apr 2024 17:55:47 +1100 (AEDT) Message-ID: <5cbb1668-1640-4a5d-b6a8-ed0e5840b9ee@wetzel-home.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=wetzel-home.de; s=wetzel-home; t=1712213733; bh=/TkSoJVhwN0sFa9XmDOyKapqxmji/XCgpaNo1YUh5MY=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=mhkEJjAi8tugEdCRcm1yf39ytSuvVtyOezlR1jntHq0B2m3i9GCfRSh9RxMW7cYJd 3tvtox0qhFFoCV6B0QP4LrVGHYgQfF/sD3Qt+EW2KbhI6VvI8pBhvxgufc7BKLK94h O+vHqg0FV/kBEnDV7hbd4ggD2HVFOYxuq6NVZATY= Date: Thu, 4 Apr 2024 08:55:30 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] scsi: sg: Avoid race in error handling & drop bogus warn To: Bart Van Assche , dgilbert@interlog.com References: <81266270-42F4-48F9-9139-8F0C3F0A6553@linux.ibm.com> <20240401191038.18359-1-Alexander@wetzel-home.de> Content-Language: en-US, de-DE From: Alexander Wetzel In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: martin.petersen@oracle.com, linux-scsi@vger.kernel.org, gregkh@linuxfoundation.org, stable@vger.kernel.org, sachinp@linux.ibm.com, linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On 04.04.24 01:24, Bart Van Assche wrote: > On 4/1/24 12:10 PM, Alexander Wetzel wrote: >> @@ -301,11 +302,12 @@ sg_open(struct inode *inode, struct file *filp) >>       /* This driver's module count bumped by fops_get in */ >>       /* Prevent the device driver from vanishing while we sleep */ >> -    retval = scsi_device_get(sdp->device); >> +    device = sdp->device; >> +    retval = scsi_device_get(device); >>       if (retval) >>           goto sg_put; > > Are all the sdp->device -> device changes essential? Isn't there a > preference to minimize patches that will end up in the stable trees? > Only the very last change is essential: - scsi_device_put(sdp->device); - goto sg_put; + kref_put(&sdp->d_ref, sg_device_destroy); + scsi_device_put(device); + return retval; Not using a (required) local variable and de-referencing it again and looks strange for anyone reading the code. While the additional lines in the patch are trivial to review... Alexander