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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 517F5C77B7E for ; Tue, 2 May 2023 11:29:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234055AbjEBL3h (ORCPT ); Tue, 2 May 2023 07:29:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43288 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233997AbjEBL3a (ORCPT ); Tue, 2 May 2023 07:29:30 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0DCC4527D for ; Tue, 2 May 2023 04:29:00 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 2C14E1F8C8; Tue, 2 May 2023 11:28:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1683026933; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=css/42tResJbpeydRWENHSWWrXYWQtFsf1y/ovDmjdA=; b=PJebdgdByAha3AwUl/EOL3aWs1RHKFUMq54D4N+O0o4+3PPk9PFKT/URND15gonhzBwT0D 8VyxK/UdeOrUnTdNhI7T6wdI7vryolgtncgSfaeVga/hkglcIBW4z5VmANYmbKnkmNV9CA JfMv/MJE2I2gO4JsvBHSAwRzvfnMrnY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1683026933; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=css/42tResJbpeydRWENHSWWrXYWQtFsf1y/ovDmjdA=; b=Zd/rgu8yvQECvxfKNKjCoHlmdkLUFFvnJZra2b9YycDPhQNS590BWcKOiBaakBCw9El9Kv uG/t/YXoZR3WO0BA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id F0BC7134FB; Tue, 2 May 2023 11:28:52 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id htT2OfTzUGSbGAAAMHmgww (envelope-from ); Tue, 02 May 2023 11:28:52 +0000 Date: Tue, 02 May 2023 13:28:52 +0200 Message-ID: <874jovotu3.wl-tiwai@suse.de> From: Takashi Iwai To: Dan Carpenter Cc: oe-kbuild@lists.linux.dev, Takashi Iwai , lkp@intel.com, oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org, Thomas Zimmermann Subject: Re: drivers/gpu/drm/udl/udl_main.c:259 udl_get_urb_locked() warn: can 'unode' even be NULL? In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0 MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 02 May 2023 13:19:04 +0200, Dan Carpenter wrote: > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > head: 2caeeb9d4a1bccd923b7918427f9e9ef7151ddd8 > commit: c5c354a3a4728045e1342166394c615d75d45377 drm/udl: Fix inconsistent urbs.count value during udl_free_urb_list() > config: parisc-randconfig-m031-20230421 (https://download.01.org/0day-ci/archive/20230423/202304230801.ncoG1XDr-lkp@intel.com/config) > compiler: hppa-linux-gcc (GCC) 12.1.0 > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot > | Reported-by: Dan Carpenter > | Link: https://lore.kernel.org/r/202304230801.ncoG1XDr-lkp@intel.com/ > > smatch warnings: > drivers/gpu/drm/udl/udl_main.c:259 udl_get_urb_locked() warn: can 'unode' even be NULL? > > vim +/unode +259 drivers/gpu/drm/udl/udl_main.c > > c5c354a3a47280 Takashi Iwai 2022-09-08 236 static struct urb *udl_get_urb_locked(struct udl_device *udl, long timeout) > 5320918b9a8786 Dave Airlie 2010-12-15 237 { > c5c354a3a47280 Takashi Iwai 2022-09-08 238 struct urb_node *unode; > 5320918b9a8786 Dave Airlie 2010-12-15 239 > c5c354a3a47280 Takashi Iwai 2022-09-08 240 assert_spin_locked(&udl->urbs.lock); > 5320918b9a8786 Dave Airlie 2010-12-15 241 > acd45c56790a3b Takashi Iwai 2022-08-04 242 /* Wait for an in-flight buffer to complete and get re-queued */ > acd45c56790a3b Takashi Iwai 2022-08-04 243 if (!wait_event_lock_irq_timeout(udl->urbs.sleep, > c5c354a3a47280 Takashi Iwai 2022-09-08 244 !udl->urbs.count || > acd45c56790a3b Takashi Iwai 2022-08-04 245 !list_empty(&udl->urbs.list), > acd45c56790a3b Takashi Iwai 2022-08-04 246 udl->urbs.lock, timeout)) { > acd45c56790a3b Takashi Iwai 2022-08-04 247 DRM_INFO("wait for urb interrupted: available: %d\n", > acd45c56790a3b Takashi Iwai 2022-08-04 248 udl->urbs.available); > c5c354a3a47280 Takashi Iwai 2022-09-08 249 return NULL; > acd45c56790a3b Takashi Iwai 2022-08-04 250 } > 5320918b9a8786 Dave Airlie 2010-12-15 251 > c5c354a3a47280 Takashi Iwai 2022-09-08 252 if (!udl->urbs.count) > c5c354a3a47280 Takashi Iwai 2022-09-08 253 return NULL; > c5c354a3a47280 Takashi Iwai 2022-09-08 254 > acd45c56790a3b Takashi Iwai 2022-08-04 255 unode = list_first_entry(&udl->urbs.list, struct urb_node, entry); > > Use list_first_entry_or_null() if this list can be empty. > > ed9605a66b62f2 Takashi Iwai 2022-09-08 256 list_del_init(&unode->entry); > ^^^^^^^^^^^^ > dereference. > > 5320918b9a8786 Dave Airlie 2010-12-15 257 udl->urbs.available--; > 5320918b9a8786 Dave Airlie 2010-12-15 258 > acd45c56790a3b Takashi Iwai 2022-08-04 @259 return unode ? unode->urb : NULL; > > Returns from list_first_entry() should never be checked for NULL. > > 5320918b9a8786 Dave Airlie 2010-12-15 260 } This is rather a leftover at commit c5c354a3a472 ("drm/udl: Fix inconsistent urbs.count value during udl_free_urb_list()"). The NULL check of unode at return became superfluous by the code change there, hence it should be simply like return ucode->urb; OTOH, the current code doesn't cause any practical problem. thanks, Takashi