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 E2B21C433F5 for ; Mon, 9 May 2022 11:49:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233331AbiEILwu (ORCPT ); Mon, 9 May 2022 07:52:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39750 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233318AbiEILws (ORCPT ); Mon, 9 May 2022 07:52:48 -0400 Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 22B7F224A5A for ; Mon, 9 May 2022 04:48:48 -0700 (PDT) Received: by mail-qk1-x735.google.com with SMTP id w3so10497945qkb.3 for ; Mon, 09 May 2022 04:48:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TXz7UBKoqgilVarq1zHbPszUf7VmazSwgELmvaxHKuw=; b=dPL+/KSAOjGmwBc6y6u4E3sq4Hqyv7q8g4JRYoASyL80EKVi/GitGdX+/PkGtcY6YB dDIk+KEfU7GWsB9Fk67FPdgQ/meSrNQyjYhYvCT5iLWr5g890S9Wzy6VOkmnSZSAoAOt 2pY7zvFP2x7isIoCnr4Oh37S235g67aUAVGn3ZRM+hAOUWeH5IBKSeCG8dUB2nxwjEHB S3OmldG/w0rgcqZssg2j+T/MTgRrScyFYqO8/A0kS2KtxXOc7THsS/G6ffHI7AZ4/57t KkkbgwPmBagHQ5XLRGhkvVZVm4mz/CkM0bIxw+D+Z1bAsODZ5t15b9T2JKvZzYJARpUG dPNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TXz7UBKoqgilVarq1zHbPszUf7VmazSwgELmvaxHKuw=; b=j0kW4/Tymcbisac712Y84NcYjqyWV0wY0VDVaiQ8wUCNb/BsaNSIISxvJoBrgLthM1 msqPQidAbZNF5qyBCBkDY+yFdh/ECB403UaQ/Mqh6uSMssE2U+0+jFaju8wqIaYZX/R5 g09vzA9YcQ8WFMSAP2gAytTq4AHxXOiZqcxatTC/8J9xU5OUVZTajvnb45ZHiuu3rY7x MS5SogV7jW94B9Gf+cu0uRBaS1UDm9ESb37Tbx6f3H1HbUq/oRYRFePlXCBhYfntLA3N okWJq0zH/KUD5wAJytGnmuoMLV8POK/KU9XOKFjCb8pXgD6MotCXccFJNDHtFSvcjtM3 sO0A== X-Gm-Message-State: AOAM5313y2gdOBhRRnN5NXlusBWCpYlEzO5YXYUkVIN6w8c1Vjge6LlQ wM9/RM00Wv55B30Pwu2JLXKxnTLAQHtOMWj8gHk= X-Google-Smtp-Source: ABdhPJzQY2Lctd9VA5YUAV8u2604rL2XWC0+gkzA0/FyJQLfAlIF5M+hQGtiyyMUrgyxGIDT3O9lAZ1QZ8edxcrcnKE= X-Received: by 2002:a37:9cd8:0:b0:69f:b618:c7ab with SMTP id f207-20020a379cd8000000b0069fb618c7abmr11556327qke.722.1652096927879; Mon, 09 May 2022 04:48:47 -0700 (PDT) MIME-Version: 1.0 References: <202205080346.m0fb3UXK-lkp@intel.com> In-Reply-To: <202205080346.m0fb3UXK-lkp@intel.com> From: Amir Goldstein Date: Mon, 9 May 2022 14:48:36 +0300 Message-ID: Subject: Re: [amir73il:fsnotify-fixes 2/2] fs/notify/fsnotify.c:540 fsnotify() warn: variable dereferenced before check 'dir1' (see line 499) To: Dan Carpenter Cc: kbuild@lists.01.org, kbuild test robot , kbuild-all@lists.01.org, linux-kernel , Matthew Bobrowski , Jan Kara Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 9, 2022 at 1:09 PM Dan Carpenter wrote: > > tree: https://github.com/amir73il/linux fsnotify-fixes > head: d25f3ce8da49ce1a3b0a0621f0bf7b1d6ba2dad6 > commit: d25f3ce8da49ce1a3b0a0621f0bf7b1d6ba2dad6 [2/2] fsnotify: send FS_RENAME to groups watching the moved inode > config: s390-randconfig-m031-20220508 (https://download.01.org/0day-ci/archive/20220508/202205080346.m0fb3UXK-lkp@intel.com/config) > compiler: s390-linux-gcc (GCC) 11.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > Reported-by: Dan Carpenter > > smatch warnings: > fs/notify/fsnotify.c:540 fsnotify() warn: variable dereferenced before check 'dir1' (see line 499) > > vim +/dir1 +540 fs/notify/fsnotify.c > > 40a100d3adc1ad Amir Goldstein 2020-07-22 475 int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir, > 40a100d3adc1ad Amir Goldstein 2020-07-22 476 const struct qstr *file_name, struct inode *inode, u32 cookie) > 90586523eb4b34 Eric Paris 2009-05-21 477 { > b54cecf5e2293d Amir Goldstein 2020-06-07 478 const struct path *path = fsnotify_data_path(data, data_type); > 29335033c574a1 Gabriel Krisman Bertazi 2021-10-25 479 struct super_block *sb = fsnotify_data_sb(data, data_type); > 3427ce71554123 Miklos Szeredi 2017-10-30 480 struct fsnotify_iter_info iter_info = {}; > 60f7ed8c7c4d06 Amir Goldstein 2018-09-01 481 struct mount *mnt = NULL; > d25f3ce8da49ce Amir Goldstein 2022-05-07 482 struct inode *dir1, *dir2; > e54183fa7047c1 Amir Goldstein 2021-11-29 483 struct dentry *moved; > d25f3ce8da49ce Amir Goldstein 2022-05-07 484 int dir1_type = 0; > 9385a84d7e1f65 Jan Kara 2016-11-10 485 int ret = 0; > 71d734103edfa2 Mel Gorman 2020-07-08 486 __u32 test_mask, marks_mask; > 90586523eb4b34 Eric Paris 2009-05-21 487 > 71d734103edfa2 Mel Gorman 2020-07-08 488 if (path) > aa93bdc5500cc9 Amir Goldstein 2020-03-19 489 mnt = real_mount(path->mnt); > 3a9fb89f4cd04c Eric Paris 2009-12-17 490 > 40a100d3adc1ad Amir Goldstein 2020-07-22 491 if (!inode) { > 40a100d3adc1ad Amir Goldstein 2020-07-22 492 /* Dirent event - report on TYPE_INODE to dir */ > 40a100d3adc1ad Amir Goldstein 2020-07-22 493 inode = dir; > d25f3ce8da49ce Amir Goldstein 2022-05-07 494 } else if (mask & FS_RENAME) { > d25f3ce8da49ce Amir Goldstein 2022-05-07 495 /* For FS_RENAME, dir1 is old_dir and dir2 is new_dir */ > e54183fa7047c1 Amir Goldstein 2021-11-29 496 moved = fsnotify_data_dentry(data, data_type); > d25f3ce8da49ce Amir Goldstein 2022-05-07 497 dir1 = moved->d_parent->d_inode; > d25f3ce8da49ce Amir Goldstein 2022-05-07 498 dir2 = dir; > d25f3ce8da49ce Amir Goldstein 2022-05-07 @499 if (dir1->i_fsnotify_marks || dir2->i_fsnotify_marks) > ^^^^^^^^^^^^^^^^^^^^^^ > Dereference > > d25f3ce8da49ce Amir Goldstein 2022-05-07 500 dir1_type = FSNOTIFY_ITER_TYPE_OLD_DIR; > d25f3ce8da49ce Amir Goldstein 2022-05-07 501 /* > d25f3ce8da49ce Amir Goldstein 2022-05-07 502 * Send FS_RENAME to groups watching the moved inode itself > d25f3ce8da49ce Amir Goldstein 2022-05-07 503 * only if the moved inode is a non-dir. > d25f3ce8da49ce Amir Goldstein 2022-05-07 504 * Sending FS_RENAME to a moved watched directory would be > d25f3ce8da49ce Amir Goldstein 2022-05-07 505 * confusing and FS_MOVE_SELF provided enough information to > d25f3ce8da49ce Amir Goldstein 2022-05-07 506 * track the movements of a watched directory. > d25f3ce8da49ce Amir Goldstein 2022-05-07 507 */ > d25f3ce8da49ce Amir Goldstein 2022-05-07 508 if (mask & FS_ISDIR) > d25f3ce8da49ce Amir Goldstein 2022-05-07 509 inode = NULL; > 40a100d3adc1ad Amir Goldstein 2020-07-22 510 } else if (mask & FS_EVENT_ON_CHILD) { > 40a100d3adc1ad Amir Goldstein 2020-07-22 511 /* > fecc4559780d52 Amir Goldstein 2020-12-02 512 * Event on child - report on TYPE_PARENT to dir if it is > fecc4559780d52 Amir Goldstein 2020-12-02 513 * watching children and on TYPE_INODE to child. > 40a100d3adc1ad Amir Goldstein 2020-07-22 514 */ > d25f3ce8da49ce Amir Goldstein 2022-05-07 515 dir1 = dir; > d25f3ce8da49ce Amir Goldstein 2022-05-07 516 dir2 = NULL; > d25f3ce8da49ce Amir Goldstein 2022-05-07 517 if (dir1->i_fsnotify_marks) > ^^^^^^^^^^^^^^^^^^^^^^ > Dereference > > d25f3ce8da49ce Amir Goldstein 2022-05-07 518 dir1_type = FSNOTIFY_ITER_TYPE_PARENT; > 40a100d3adc1ad Amir Goldstein 2020-07-22 519 } > 497b0c5a7c0688 Amir Goldstein 2020-07-16 520 > 7c49b8616460eb Dave Hansen 2015-09-04 521 /* > 7c49b8616460eb Dave Hansen 2015-09-04 522 * Optimization: srcu_read_lock() has a memory barrier which can > 7c49b8616460eb Dave Hansen 2015-09-04 523 * be expensive. It protects walking the *_fsnotify_marks lists. > 7c49b8616460eb Dave Hansen 2015-09-04 524 * However, if we do not walk the lists, we do not have to do > 7c49b8616460eb Dave Hansen 2015-09-04 525 * SRCU because we have no references to any objects and do not > 7c49b8616460eb Dave Hansen 2015-09-04 526 * need SRCU to keep them "alive". > 7c49b8616460eb Dave Hansen 2015-09-04 527 */ > 9b93f33105f5f9 Amir Goldstein 2020-07-16 528 if (!sb->s_fsnotify_marks && > 497b0c5a7c0688 Amir Goldstein 2020-07-16 529 (!mnt || !mnt->mnt_fsnotify_marks) && > 9b93f33105f5f9 Amir Goldstein 2020-07-16 530 (!inode || !inode->i_fsnotify_marks) && > d25f3ce8da49ce Amir Goldstein 2022-05-07 531 !dir1_type) > 7c49b8616460eb Dave Hansen 2015-09-04 532 return 0; > 71d734103edfa2 Mel Gorman 2020-07-08 533 > 9b93f33105f5f9 Amir Goldstein 2020-07-16 534 marks_mask = sb->s_fsnotify_mask; > 71d734103edfa2 Mel Gorman 2020-07-08 535 if (mnt) > 71d734103edfa2 Mel Gorman 2020-07-08 536 marks_mask |= mnt->mnt_fsnotify_mask; > 9b93f33105f5f9 Amir Goldstein 2020-07-16 537 if (inode) > 9b93f33105f5f9 Amir Goldstein 2020-07-16 538 marks_mask |= inode->i_fsnotify_mask; > d25f3ce8da49ce Amir Goldstein 2022-05-07 539 if (dir1_type) { > d25f3ce8da49ce Amir Goldstein 2022-05-07 @540 if (dir1) > > If "dir1_type" is set then we have already dereferenced "dir1". I guess > this unnecessary NULL check is something that probably wouldn't bother > a human reader too much... True !!dir1_type means !!dir1. We could get rid of if (dir1) but I should note that I did not post this patch for review, this is an untested patch that I pushed to a branch on my private github. It is very generous of kbuild testbot to test any branch I push to my private github, but as much as I can recall, I never asked for it... Thanks, Amir. From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============0596045951092484093==" MIME-Version: 1.0 From: Amir Goldstein To: kbuild-all@lists.01.org Subject: Re: [amir73il:fsnotify-fixes 2/2] fs/notify/fsnotify.c:540 fsnotify() warn: variable dereferenced before check 'dir1' (see line 499) Date: Mon, 09 May 2022 14:48:36 +0300 Message-ID: In-Reply-To: <202205080346.m0fb3UXK-lkp@intel.com> List-Id: --===============0596045951092484093== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Mon, May 9, 2022 at 1:09 PM Dan Carpenter w= rote: > > tree: https://github.com/amir73il/linux fsnotify-fixes > head: d25f3ce8da49ce1a3b0a0621f0bf7b1d6ba2dad6 > commit: d25f3ce8da49ce1a3b0a0621f0bf7b1d6ba2dad6 [2/2] fsnotify: send FS_= RENAME to groups watching the moved inode > config: s390-randconfig-m031-20220508 (https://download.01.org/0day-ci/ar= chive/20220508/202205080346.m0fb3UXK-lkp(a)intel.com/config) > compiler: s390-linux-gcc (GCC) 11.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > Reported-by: Dan Carpenter > > smatch warnings: > fs/notify/fsnotify.c:540 fsnotify() warn: variable dereferenced before ch= eck 'dir1' (see line 499) > > vim +/dir1 +540 fs/notify/fsnotify.c > > 40a100d3adc1ad Amir Goldstein 2020-07-22 475 int fsnotify(__u3= 2 mask, const void *data, int data_type, struct inode *dir, > 40a100d3adc1ad Amir Goldstein 2020-07-22 476 cons= t struct qstr *file_name, struct inode *inode, u32 cookie) > 90586523eb4b34 Eric Paris 2009-05-21 477 { > b54cecf5e2293d Amir Goldstein 2020-06-07 478 const str= uct path *path =3D fsnotify_data_path(data, data_type); > 29335033c574a1 Gabriel Krisman Bertazi 2021-10-25 479 struct su= per_block *sb =3D fsnotify_data_sb(data, data_type); > 3427ce71554123 Miklos Szeredi 2017-10-30 480 struct fs= notify_iter_info iter_info =3D {}; > 60f7ed8c7c4d06 Amir Goldstein 2018-09-01 481 struct mo= unt *mnt =3D NULL; > d25f3ce8da49ce Amir Goldstein 2022-05-07 482 struct in= ode *dir1, *dir2; > e54183fa7047c1 Amir Goldstein 2021-11-29 483 struct de= ntry *moved; > d25f3ce8da49ce Amir Goldstein 2022-05-07 484 int dir1_= type =3D 0; > 9385a84d7e1f65 Jan Kara 2016-11-10 485 int ret = =3D 0; > 71d734103edfa2 Mel Gorman 2020-07-08 486 __u32 tes= t_mask, marks_mask; > 90586523eb4b34 Eric Paris 2009-05-21 487 > 71d734103edfa2 Mel Gorman 2020-07-08 488 if (path) > aa93bdc5500cc9 Amir Goldstein 2020-03-19 489 m= nt =3D real_mount(path->mnt); > 3a9fb89f4cd04c Eric Paris 2009-12-17 490 > 40a100d3adc1ad Amir Goldstein 2020-07-22 491 if (!inod= e) { > 40a100d3adc1ad Amir Goldstein 2020-07-22 492 /= * Dirent event - report on TYPE_INODE to dir */ > 40a100d3adc1ad Amir Goldstein 2020-07-22 493 i= node =3D dir; > d25f3ce8da49ce Amir Goldstein 2022-05-07 494 } else if= (mask & FS_RENAME) { > d25f3ce8da49ce Amir Goldstein 2022-05-07 495 /= * For FS_RENAME, dir1 is old_dir and dir2 is new_dir */ > e54183fa7047c1 Amir Goldstein 2021-11-29 496 m= oved =3D fsnotify_data_dentry(data, data_type); > d25f3ce8da49ce Amir Goldstein 2022-05-07 497 d= ir1 =3D moved->d_parent->d_inode; > d25f3ce8da49ce Amir Goldstein 2022-05-07 498 d= ir2 =3D dir; > d25f3ce8da49ce Amir Goldstein 2022-05-07 @499 i= f (dir1->i_fsnotify_marks || dir2->i_fsnotify_marks) > = ^^^^^^^^^^^^^^^^^^^^^^ > Dereference > > d25f3ce8da49ce Amir Goldstein 2022-05-07 500 = dir1_type =3D FSNOTIFY_ITER_TYPE_OLD_DIR; > d25f3ce8da49ce Amir Goldstein 2022-05-07 501 /* > d25f3ce8da49ce Amir Goldstein 2022-05-07 502 = * Send FS_RENAME to groups watching the moved inode itself > d25f3ce8da49ce Amir Goldstein 2022-05-07 503 = * only if the moved inode is a non-dir. > d25f3ce8da49ce Amir Goldstein 2022-05-07 504 = * Sending FS_RENAME to a moved watched directory would be > d25f3ce8da49ce Amir Goldstein 2022-05-07 505 = * confusing and FS_MOVE_SELF provided enough information to > d25f3ce8da49ce Amir Goldstein 2022-05-07 506 = * track the movements of a watched directory. > d25f3ce8da49ce Amir Goldstein 2022-05-07 507 = */ > d25f3ce8da49ce Amir Goldstein 2022-05-07 508 i= f (mask & FS_ISDIR) > d25f3ce8da49ce Amir Goldstein 2022-05-07 509 = inode =3D NULL; > 40a100d3adc1ad Amir Goldstein 2020-07-22 510 } else if= (mask & FS_EVENT_ON_CHILD) { > 40a100d3adc1ad Amir Goldstein 2020-07-22 511 /* > fecc4559780d52 Amir Goldstein 2020-12-02 512 = * Event on child - report on TYPE_PARENT to dir if it is > fecc4559780d52 Amir Goldstein 2020-12-02 513 = * watching children and on TYPE_INODE to child. > 40a100d3adc1ad Amir Goldstein 2020-07-22 514 = */ > d25f3ce8da49ce Amir Goldstein 2022-05-07 515 d= ir1 =3D dir; > d25f3ce8da49ce Amir Goldstein 2022-05-07 516 d= ir2 =3D NULL; > d25f3ce8da49ce Amir Goldstein 2022-05-07 517 i= f (dir1->i_fsnotify_marks) > = ^^^^^^^^^^^^^^^^^^^^^^ > Dereference > > d25f3ce8da49ce Amir Goldstein 2022-05-07 518 = dir1_type =3D FSNOTIFY_ITER_TYPE_PARENT; > 40a100d3adc1ad Amir Goldstein 2020-07-22 519 } > 497b0c5a7c0688 Amir Goldstein 2020-07-16 520 > 7c49b8616460eb Dave Hansen 2015-09-04 521 /* > 7c49b8616460eb Dave Hansen 2015-09-04 522 * Optimi= zation: srcu_read_lock() has a memory barrier which can > 7c49b8616460eb Dave Hansen 2015-09-04 523 * be exp= ensive. It protects walking the *_fsnotify_marks lists. > 7c49b8616460eb Dave Hansen 2015-09-04 524 * Howeve= r, if we do not walk the lists, we do not have to do > 7c49b8616460eb Dave Hansen 2015-09-04 525 * SRCU b= ecause we have no references to any objects and do not > 7c49b8616460eb Dave Hansen 2015-09-04 526 * need S= RCU to keep them "alive". > 7c49b8616460eb Dave Hansen 2015-09-04 527 */ > 9b93f33105f5f9 Amir Goldstein 2020-07-16 528 if (!sb->= s_fsnotify_marks && > 497b0c5a7c0688 Amir Goldstein 2020-07-16 529 (!mnt= || !mnt->mnt_fsnotify_marks) && > 9b93f33105f5f9 Amir Goldstein 2020-07-16 530 (!ino= de || !inode->i_fsnotify_marks) && > d25f3ce8da49ce Amir Goldstein 2022-05-07 531 !dir1= _type) > 7c49b8616460eb Dave Hansen 2015-09-04 532 r= eturn 0; > 71d734103edfa2 Mel Gorman 2020-07-08 533 > 9b93f33105f5f9 Amir Goldstein 2020-07-16 534 marks_mas= k =3D sb->s_fsnotify_mask; > 71d734103edfa2 Mel Gorman 2020-07-08 535 if (mnt) > 71d734103edfa2 Mel Gorman 2020-07-08 536 m= arks_mask |=3D mnt->mnt_fsnotify_mask; > 9b93f33105f5f9 Amir Goldstein 2020-07-16 537 if (inode) > 9b93f33105f5f9 Amir Goldstein 2020-07-16 538 m= arks_mask |=3D inode->i_fsnotify_mask; > d25f3ce8da49ce Amir Goldstein 2022-05-07 539 if (dir1_= type) { > d25f3ce8da49ce Amir Goldstein 2022-05-07 @540 i= f (dir1) > > If "dir1_type" is set then we have already dereferenced "dir1". I guess > this unnecessary NULL check is something that probably wouldn't bother > a human reader too much... True !!dir1_type means !!dir1. We could get rid of if (dir1) but I should note that I did not post this patch for review, this is an unt= ested patch that I pushed to a branch on my private github. It is very generous of kbuild testbot to test any branch I push to my private github, but as much as I can recall, I never asked for it... Thanks, Amir. --===============0596045951092484093==--