Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <be59d5ac-fabb-4a05-99d4-fb281f7ea507@intel.com>
Date:   Wed, 13 Dec 2023 09:55:08 -0700
User-Agent: Betterbird (Linux)
Subject: Re: [PATCH] dmaengine: fix NULL pointer in channel unregistration
 function
To:     Amelie Delaunay <amelie.delaunay@foss.st.com>,
        Vinod Koul <vkoul@kernel.org>
CC:     <linux-stm32@st-md-mailman.stormreply.com>,
        <dmaengine@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20231213160452.2598073-1-amelie.delaunay@foss.st.com>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20231213160452.2598073-1-amelie.delaunay@foss.st.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SHd2a2N4UFFsQ3Bud09vNE1iUU4yQWdmM2NJclZYSnlpRjR0L0dkSTVsSksv?=
 =?utf-8?B?MWF2MTIyb3JZYXJuRDlyUTZXLzc1TnRjWk1HVjNFWW4vSWdrMk9hNWQzbTZt?=
 =?utf-8?B?dEdCSkE3WGJMdzM3UmhyUUpZU01ORHNPKzNLUTV5c2U2ZitrTXBCOWxJK0Ft?=
 =?utf-8?B?Q0l1bTVJbWEyOUhQYkI1N0FESEMzZWRYMUpCQlZoeXFVclRlUkJYUGRhRFFa?=
 =?utf-8?B?RW00TUNGdXZTRSsyWlRlZi95V1ljYjJ5Y2R5YmJQd2Y0Yk51M2UrMHVuSits?=
 =?utf-8?B?c3I4Y2Z6aGpvK1FuVEt6cnhBV0U1R1U4VkhBbDV5VmlLQVp3MUswN2xOQnQx?=
 =?utf-8?B?K2F3MEtyU1dQRlZmd2xMTFhON1dYRW5oZ2lJa0dIL3U1U0t4UU5Ta3VjaDJm?=
 =?utf-8?B?bFc2MFJXUGt0MlVjWHlZeEQ1Y0pZc3lVRjFFRjV5eDBudU40aHdpTEVWYUY1?=
 =?utf-8?B?SkxCSTBWRE1oOVRGWmNscXdZQXo4eFp4cGFpS296U3hEN1VSd2VUemorZzdh?=
 =?utf-8?B?QVJKWThtSjk1SkZFWDJyL0JyUHJNZno1RUZlbkVadTBxYlhhTTNsTmRuc1B5?=
 =?utf-8?B?S2tIOXdQWStXbm5uWGF6SG5RQVJNbG15RVRtdjJFb0ptdEVxVGJ4YnNhdnpF?=
 =?utf-8?B?RDFVWWxhYXFrVXRKQmNQZmZRK1FEalNKamkreHNEUCt5dHJlZ0pIYndERzkx?=
 =?utf-8?B?VG93SUlmMTVXekJXWHlCZWF4VDBVWlRoZHBpaGtmdFRPc29sQ3FwSlI5L3N4?=
 =?utf-8?B?Rk9nYlZSSmxtQVJCVHlTZGoreElHMUNwbWVJTE1NMTRRb2NpT2o4czBVQnQr?=
 =?utf-8?B?dWNqemh5bG4xTG4vdU5WQTFmTXpVbExNK2pUeWwxZkJsZGV4blVvWmx4YVIv?=
 =?utf-8?B?S0VLQ2pJOEZ5VmtDNHRSUkZ0Z0lCRUxveElGTld0TW8wYWlRb2RQMW1PVjZ6?=
 =?utf-8?B?NkVDc2JXK1VIVkpIcUQxQXNaU1NlRS81WkI0Um9YS2l6RTF4NGtKcHZXdWRH?=
 =?utf-8?B?V1JGckl1TVVJNGhLalEveUozMDR4Z2EwZ3JRZXByN0dNWkg1QVVtZmt5QThW?=
 =?utf-8?B?dkM2Ykcxa0JBeTFzUW5nWWZMa2tDb1U4Wk9GRzFBdzdYMDJVbSszN2NLRUFu?=
 =?utf-8?B?ZCtXTlhNemFrdGpwcW5XaHV5RnF5VmtWdTlhMlR2T3pzUTN1cW5saFk0aHBE?=
 =?utf-8?B?MHlOQ0JFbERBVThJZ1RBZk0yWkZXemt4V3pmVFFMYXRkTURWRHpVRVRDQjhP?=
 =?utf-8?B?OVE0VnJZMnZITWpJR3p2T1RVL3UyWnZIaEFEYmpIRUZ5T0hqN3dBNE5LdTFI?=
 =?utf-8?B?dEo1L0RDajBHb2F3Y2FUQTFieEwvR3NOaHlFMHR0Z3ZSN0hzQSt0ZHV5R0N6?=
 =?utf-8?B?VjZRcVhzeFEweHYwZDZjVVZIaDQvQzN2K0haL2dEbDc1bEF4VHlzZENsUFVC?=
 =?utf-8?B?YnljelBiWFFvYTBIbFpQYVV4bXdYckxxSHhsZnhPcTVRZmtPRS9FZFJYbUZu?=
 =?utf-8?B?aHhxWUg1R1BVcW9JMlNNY3dWTmQ0SDF3VXEwcGpsRUFTTkpJMzRXeGtTUDVj?=
 =?utf-8?B?SW96L0JnUk0xeDNRMVl0UWRQZVkvT2hOY3FkcGh2QXZ5Ym9venArajRXZ3hm?=
 =?utf-8?B?SHQrZ0U2MnlET1U1bFJMclBrVFFFRTQ4MVYvcHhDUnM5ME1OMTcrVCtTTnpi?=
 =?utf-8?B?ZWoyYTZwZzUwRFU2ZVRGVDlDbDNpc2prUWVpRmszV3kvZWdkUWNIQWNHcDI1?=
 =?utf-8?B?aUVwdXpSL3d0MnZhT3pLMENudVRMYmRneUl0TVJlYVp6TS8wanBHVXJuOXNI?=
 =?utf-8?B?NEZ6aTEzTGt0eFFEZHpzWVk1SnZrbFM2OFFmNTNENkhHWnkwQytHaFRKZ0FE?=
 =?utf-8?B?Mzk3TFVodFlJQWg1VFVjeExZN29zekNiMThPSTA4MU0rbWVTSkc2VWt0Tjhh?=
 =?utf-8?B?dVVzcVVKTmFSRi9GTnN0cGlCbnBQM0NEdVZQSGRqQmgrSW16K2NCMnRTK1Ja?=
 =?utf-8?B?TVlDdDVIa2p2QTQrYXd1R09jdVMvaFNPamhRQmF4dWxnVzU0MzdiS1JBWVVC?=
 =?utf-8?B?Q1ZwOUVOMUJBdXUrSDYzS3B3RVl3V3J2dlQ4MzVhbS9xaVBVWkU0dW40QmVP?=
 =?utf-8?Q?SHkfH0cm2UlHxbRHp/nSTpiyp?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5596d7ac-654b-4b78-8e42-08dbfbfc4571
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 16:55:12.0801
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wjYGiwlLCROQp538OSVKKHYoQ+9geoljcDu0kKxTjQ439yaQfwFvHE6Q1cgd30XOZwPCmWMrgclpgGhC+0d1bQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB7117
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Wed, 13 Dec 2023 08:55:32 -0800 (PST)


On 12/13/23 09:04, Amelie Delaunay wrote:
> __dma_async_device_channel_register() can fail. In case of failure,
> chan->local is freed (with free_percpu()), and chan->local is nullified.
> When dma_async_device_unregister() is called (because of managed API or
> intentionally by DMA controller driver), channels are unconditionally
> unregistered, leading to this NULL pointer:
> [    1.318693] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000d0
> [...]
> [    1.484499] Call trace:
> [    1.486930]  device_del+0x40/0x394
> [    1.490314]  device_unregister+0x20/0x7c
> [    1.494220]  __dma_async_device_channel_unregister+0x68/0xc0
> 
> Look at dma_async_device_register() function error path, channel device
> unregistration is done only if chan->local is not NULL.
> 
> Then add the same condition at the beginning of
> __dma_async_device_channel_unregister() function, to avoid NULL pointer
> issue whatever the API used to reach this function.
> 
> Fixes: d2fb0a043838 ("dmaengine: break out channel registration")
> Signed-off-by: Amelie Delaunay <amelie.delaunay@foss.st.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dma/dmaengine.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index b7388ae62d7f..491b22240221 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -1103,6 +1103,9 @@ EXPORT_SYMBOL_GPL(dma_async_device_channel_register);
>  static void __dma_async_device_channel_unregister(struct dma_device *device,
>  						  struct dma_chan *chan)
>  {
> +	if (chan->local == NULL)
> +		return;
> +
>  	WARN_ONCE(!device->device_release && chan->client_count,
>  		  "%s called while %d clients hold a reference\n",
>  		  __func__, chan->client_count);