public inbox for llvm@lists.linux.dev
 help / color / mirror / Atom feed
* Re: [PATCH v4 2/4] fanotify: define struct members to hold response decision context
       [not found] <8767f3a0d43d6a994584b86c03eb659a662cc416.1659996830.git.rgb@redhat.com>
@ 2022-08-10 14:28 ` kernel test robot
  2022-08-19 16:25   ` Richard Guy Briggs
  0 siblings, 1 reply; 4+ messages in thread
From: kernel test robot @ 2022-08-10 14:28 UTC (permalink / raw)
  To: Richard Guy Briggs, Linux-Audit Mailing List, LKML, linux-fsdevel
  Cc: llvm, kbuild-all, Paul Moore, Eric Paris, Steve Grubb,
	Richard Guy Briggs, Jan Kara, Amir Goldstein

Hi Richard,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jack-fs/fsnotify]
[also build test WARNING on pcmoore-audit/next linus/master v5.19 next-20220810]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220810/202208102231.qSUdYAdb-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/a943676abc023c094f05b45f4d61936c567507a2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
        git checkout a943676abc023c094f05b45f4d61936c567507a2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/notify/fanotify/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/notify/fanotify/fanotify_user.c:325:35: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
                    group, fd, response, info_buf, count);
                                                   ^~~~~
   include/linux/printk.h:594:38: note: expanded from macro 'pr_debug'
           no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
                                       ~~~     ^~~~~~~~~~~
   include/linux/printk.h:131:17: note: expanded from macro 'no_printk'
                   printk(fmt, ##__VA_ARGS__);             \
                          ~~~    ^~~~~~~~~~~
   include/linux/printk.h:464:60: note: expanded from macro 'printk'
   #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
                                                       ~~~    ^~~~~~~~~~~
   include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
                   _p_func(_fmt, ##__VA_ARGS__);                           \
                           ~~~~    ^~~~~~~~~~~
   1 warning generated.


vim +325 fs/notify/fanotify/fanotify_user.c

   312	
   313	static int process_access_response(struct fsnotify_group *group,
   314					   struct fanotify_response *response_struct,
   315					   const char __user *buf,
   316					   size_t count)
   317	{
   318		struct fanotify_perm_event *event;
   319		int fd = response_struct->fd;
   320		u32 response = response_struct->response;
   321		struct fanotify_response_info_header info_hdr;
   322		char *info_buf = NULL;
   323	
   324		pr_debug("%s: group=%p fd=%d response=%u buf=%p size=%lu\n", __func__,
 > 325			 group, fd, response, info_buf, count);
   326		/*
   327		 * make sure the response is valid, if invalid we do nothing and either
   328		 * userspace can send a valid response or we will clean it up after the
   329		 * timeout
   330		 */
   331		if (response & ~FANOTIFY_RESPONSE_VALID_MASK)
   332			return -EINVAL;
   333		switch (response & FANOTIFY_RESPONSE_ACCESS) {
   334		case FAN_ALLOW:
   335		case FAN_DENY:
   336			break;
   337		default:
   338			return -EINVAL;
   339		}
   340		if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
   341			return -EINVAL;
   342		if (fd < 0)
   343			return -EINVAL;
   344		if (response & FAN_INFO) {
   345			size_t c = count;
   346			const char __user *ib = buf;
   347	
   348			if (c <= 0)
   349				return -EINVAL;
   350			while (c >= sizeof(info_hdr)) {
   351				if (copy_from_user(&info_hdr, ib, sizeof(info_hdr)))
   352					return -EFAULT;
   353				if (info_hdr.pad != 0)
   354					return -EINVAL;
   355				if (c < info_hdr.len)
   356					return -EINVAL;
   357				switch (info_hdr.type) {
   358				case FAN_RESPONSE_INFO_AUDIT_RULE:
   359					break;
   360				case FAN_RESPONSE_INFO_NONE:
   361				default:
   362					return -EINVAL;
   363				}
   364				c -= info_hdr.len;
   365				ib += info_hdr.len;
   366			}
   367			if (c != 0)
   368				return -EINVAL;
   369			/* Simplistic check for now */
   370			if (count != sizeof(struct fanotify_response_info_audit_rule))
   371				return -EINVAL;
   372			info_buf = kmalloc(sizeof(struct fanotify_response_info_audit_rule),
   373					   GFP_KERNEL);
   374			if (!info_buf)
   375				return -ENOMEM;
   376			if (copy_from_user(info_buf, buf, count))
   377				return -EFAULT;
   378		}
   379		spin_lock(&group->notification_lock);
   380		list_for_each_entry(event, &group->fanotify_data.access_list,
   381				    fae.fse.list) {
   382			if (event->fd != fd)
   383				continue;
   384	
   385			list_del_init(&event->fae.fse.list);
   386			/* finish_permission_event() eats info_buf */
   387			finish_permission_event(group, event, response_struct,
   388						count, info_buf);
   389			wake_up(&group->fanotify_data.access_waitq);
   390			return 0;
   391		}
   392		spin_unlock(&group->notification_lock);
   393	
   394		return -ENOENT;
   395	}
   396	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 2/4] fanotify: define struct members to hold response decision context
  2022-08-10 14:28 ` [PATCH v4 2/4] fanotify: define struct members to hold response decision context kernel test robot
@ 2022-08-19 16:25   ` Richard Guy Briggs
  2022-08-19 17:17     ` Nick Desaulniers
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Guy Briggs @ 2022-08-19 16:25 UTC (permalink / raw)
  To: kernel test robot
  Cc: Linux-Audit Mailing List, LKML, linux-fsdevel, llvm, kbuild-all,
	Paul Moore, Eric Paris, Steve Grubb, Jan Kara, Amir Goldstein

On 2022-08-10 22:28, kernel test robot wrote:
> Hi Richard,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on jack-fs/fsnotify]
> [also build test WARNING on pcmoore-audit/next linus/master v5.19 next-20220810]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
> config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220810/202208102231.qSUdYAdb-lkp@intel.com/config)
> compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/a943676abc023c094f05b45f4d61936c567507a2
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
>         git checkout a943676abc023c094f05b45f4d61936c567507a2
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/notify/fanotify/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> >> fs/notify/fanotify/fanotify_user.c:325:35: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]

Interesting.  When I "fix" it, my compiler complains:

	fs/notify/fanotify/fanotify_user.c:324:11: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 8 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]

>                     group, fd, response, info_buf, count);
>                                                    ^~~~~
>    include/linux/printk.h:594:38: note: expanded from macro 'pr_debug'
>            no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
>                                        ~~~     ^~~~~~~~~~~
>    include/linux/printk.h:131:17: note: expanded from macro 'no_printk'
>                    printk(fmt, ##__VA_ARGS__);             \
>                           ~~~    ^~~~~~~~~~~
>    include/linux/printk.h:464:60: note: expanded from macro 'printk'
>    #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
>                                                        ~~~    ^~~~~~~~~~~
>    include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
>                    _p_func(_fmt, ##__VA_ARGS__);                           \
>                            ~~~~    ^~~~~~~~~~~
>    1 warning generated.
> 
> 
> vim +325 fs/notify/fanotify/fanotify_user.c
> 
>    312	
>    313	static int process_access_response(struct fsnotify_group *group,
>    314					   struct fanotify_response *response_struct,
>    315					   const char __user *buf,
>    316					   size_t count)
>    317	{
>    318		struct fanotify_perm_event *event;
>    319		int fd = response_struct->fd;
>    320		u32 response = response_struct->response;
>    321		struct fanotify_response_info_header info_hdr;
>    322		char *info_buf = NULL;
>    323	
>    324		pr_debug("%s: group=%p fd=%d response=%u buf=%p size=%lu\n", __func__,
>  > 325			 group, fd, response, info_buf, count);
>    326		/*
>    327		 * make sure the response is valid, if invalid we do nothing and either
>    328		 * userspace can send a valid response or we will clean it up after the
>    329		 * timeout
>    330		 */
>    331		if (response & ~FANOTIFY_RESPONSE_VALID_MASK)
>    332			return -EINVAL;
>    333		switch (response & FANOTIFY_RESPONSE_ACCESS) {
>    334		case FAN_ALLOW:
>    335		case FAN_DENY:
>    336			break;
>    337		default:
>    338			return -EINVAL;
>    339		}
>    340		if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
>    341			return -EINVAL;
>    342		if (fd < 0)
>    343			return -EINVAL;
>    344		if (response & FAN_INFO) {
>    345			size_t c = count;
>    346			const char __user *ib = buf;
>    347	
>    348			if (c <= 0)
>    349				return -EINVAL;
>    350			while (c >= sizeof(info_hdr)) {
>    351				if (copy_from_user(&info_hdr, ib, sizeof(info_hdr)))
>    352					return -EFAULT;
>    353				if (info_hdr.pad != 0)
>    354					return -EINVAL;
>    355				if (c < info_hdr.len)
>    356					return -EINVAL;
>    357				switch (info_hdr.type) {
>    358				case FAN_RESPONSE_INFO_AUDIT_RULE:
>    359					break;
>    360				case FAN_RESPONSE_INFO_NONE:
>    361				default:
>    362					return -EINVAL;
>    363				}
>    364				c -= info_hdr.len;
>    365				ib += info_hdr.len;
>    366			}
>    367			if (c != 0)
>    368				return -EINVAL;
>    369			/* Simplistic check for now */
>    370			if (count != sizeof(struct fanotify_response_info_audit_rule))
>    371				return -EINVAL;
>    372			info_buf = kmalloc(sizeof(struct fanotify_response_info_audit_rule),
>    373					   GFP_KERNEL);
>    374			if (!info_buf)
>    375				return -ENOMEM;
>    376			if (copy_from_user(info_buf, buf, count))
>    377				return -EFAULT;
>    378		}
>    379		spin_lock(&group->notification_lock);
>    380		list_for_each_entry(event, &group->fanotify_data.access_list,
>    381				    fae.fse.list) {
>    382			if (event->fd != fd)
>    383				continue;
>    384	
>    385			list_del_init(&event->fae.fse.list);
>    386			/* finish_permission_event() eats info_buf */
>    387			finish_permission_event(group, event, response_struct,
>    388						count, info_buf);
>    389			wake_up(&group->fanotify_data.access_waitq);
>    390			return 0;
>    391		}
>    392		spin_unlock(&group->notification_lock);
>    393	
>    394		return -ENOENT;
>    395	}
>    396	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 2/4] fanotify: define struct members to hold response decision context
  2022-08-19 16:25   ` Richard Guy Briggs
@ 2022-08-19 17:17     ` Nick Desaulniers
  2022-08-19 21:45       ` Richard Guy Briggs
  0 siblings, 1 reply; 4+ messages in thread
From: Nick Desaulniers @ 2022-08-19 17:17 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: kernel test robot, Linux-Audit Mailing List, LKML, linux-fsdevel,
	llvm, kbuild-all, Paul Moore, Eric Paris, Steve Grubb, Jan Kara,
	Amir Goldstein

On Fri, Aug 19, 2022 at 9:25 AM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> On 2022-08-10 22:28, kernel test robot wrote:
> > Hi Richard,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on jack-fs/fsnotify]
> > [also build test WARNING on pcmoore-audit/next linus/master v5.19 next-20220810]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url:    https://github.com/intel-lab-lkp/linux/commits/Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
> > config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220810/202208102231.qSUdYAdb-lkp@intel.com/config)
> > compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # https://github.com/intel-lab-lkp/linux/commit/a943676abc023c094f05b45f4d61936c567507a2
> >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
> >         git checkout a943676abc023c094f05b45f4d61936c567507a2
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/notify/fanotify/
> >
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> fs/notify/fanotify/fanotify_user.c:325:35: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
>
> Interesting.  When I "fix" it, my compiler complains:
>
>         fs/notify/fanotify/fanotify_user.c:324:11: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 8 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]

The correct format specifier for size_t is %zu.  This avoids issues
between ILP32 vs LP64 targets.

>
> >                     group, fd, response, info_buf, count);
> >                                                    ^~~~~
> >    include/linux/printk.h:594:38: note: expanded from macro 'pr_debug'
> >            no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> >                                        ~~~     ^~~~~~~~~~~
> >    include/linux/printk.h:131:17: note: expanded from macro 'no_printk'
> >                    printk(fmt, ##__VA_ARGS__);             \
> >                           ~~~    ^~~~~~~~~~~
> >    include/linux/printk.h:464:60: note: expanded from macro 'printk'
> >    #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> >                                                        ~~~    ^~~~~~~~~~~
> >    include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
> >                    _p_func(_fmt, ##__VA_ARGS__);                           \
> >                            ~~~~    ^~~~~~~~~~~
> >    1 warning generated.
> >
> >
> > vim +325 fs/notify/fanotify/fanotify_user.c
> >
> >    312
> >    313        static int process_access_response(struct fsnotify_group *group,
> >    314                                           struct fanotify_response *response_struct,
> >    315                                           const char __user *buf,
> >    316                                           size_t count)
> >    317        {
> >    318                struct fanotify_perm_event *event;
> >    319                int fd = response_struct->fd;
> >    320                u32 response = response_struct->response;
> >    321                struct fanotify_response_info_header info_hdr;
> >    322                char *info_buf = NULL;
> >    323
> >    324                pr_debug("%s: group=%p fd=%d response=%u buf=%p size=%lu\n", __func__,
> >  > 325                         group, fd, response, info_buf, count);
> >    326                /*
> >    327                 * make sure the response is valid, if invalid we do nothing and either
> >    328                 * userspace can send a valid response or we will clean it up after the
> >    329                 * timeout
> >    330                 */
> >    331                if (response & ~FANOTIFY_RESPONSE_VALID_MASK)
> >    332                        return -EINVAL;
> >    333                switch (response & FANOTIFY_RESPONSE_ACCESS) {
> >    334                case FAN_ALLOW:
> >    335                case FAN_DENY:
> >    336                        break;
> >    337                default:
> >    338                        return -EINVAL;
> >    339                }
> >    340                if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
> >    341                        return -EINVAL;
> >    342                if (fd < 0)
> >    343                        return -EINVAL;
> >    344                if (response & FAN_INFO) {
> >    345                        size_t c = count;
> >    346                        const char __user *ib = buf;
> >    347
> >    348                        if (c <= 0)
> >    349                                return -EINVAL;
> >    350                        while (c >= sizeof(info_hdr)) {
> >    351                                if (copy_from_user(&info_hdr, ib, sizeof(info_hdr)))
> >    352                                        return -EFAULT;
> >    353                                if (info_hdr.pad != 0)
> >    354                                        return -EINVAL;
> >    355                                if (c < info_hdr.len)
> >    356                                        return -EINVAL;
> >    357                                switch (info_hdr.type) {
> >    358                                case FAN_RESPONSE_INFO_AUDIT_RULE:
> >    359                                        break;
> >    360                                case FAN_RESPONSE_INFO_NONE:
> >    361                                default:
> >    362                                        return -EINVAL;
> >    363                                }
> >    364                                c -= info_hdr.len;
> >    365                                ib += info_hdr.len;
> >    366                        }
> >    367                        if (c != 0)
> >    368                                return -EINVAL;
> >    369                        /* Simplistic check for now */
> >    370                        if (count != sizeof(struct fanotify_response_info_audit_rule))
> >    371                                return -EINVAL;
> >    372                        info_buf = kmalloc(sizeof(struct fanotify_response_info_audit_rule),
> >    373                                           GFP_KERNEL);
> >    374                        if (!info_buf)
> >    375                                return -ENOMEM;
> >    376                        if (copy_from_user(info_buf, buf, count))
> >    377                                return -EFAULT;
> >    378                }
> >    379                spin_lock(&group->notification_lock);
> >    380                list_for_each_entry(event, &group->fanotify_data.access_list,
> >    381                                    fae.fse.list) {
> >    382                        if (event->fd != fd)
> >    383                                continue;
> >    384
> >    385                        list_del_init(&event->fae.fse.list);
> >    386                        /* finish_permission_event() eats info_buf */
> >    387                        finish_permission_event(group, event, response_struct,
> >    388                                                count, info_buf);
> >    389                        wake_up(&group->fanotify_data.access_waitq);
> >    390                        return 0;
> >    391                }
> >    392                spin_unlock(&group->notification_lock);
> >    393
> >    394                return -ENOENT;
> >    395        }
> >    396
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://01.org/lkp
> >
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
>
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 2/4] fanotify: define struct members to hold response decision context
  2022-08-19 17:17     ` Nick Desaulniers
@ 2022-08-19 21:45       ` Richard Guy Briggs
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Guy Briggs @ 2022-08-19 21:45 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: kernel test robot, Linux-Audit Mailing List, LKML, linux-fsdevel,
	llvm, kbuild-all, Paul Moore, Eric Paris, Steve Grubb, Jan Kara,
	Amir Goldstein

On 2022-08-19 10:17, Nick Desaulniers wrote:
> On Fri, Aug 19, 2022 at 9:25 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > On 2022-08-10 22:28, kernel test robot wrote:
> > > Hi Richard,
> > >
> > > Thank you for the patch! Perhaps something to improve:
> > >
> > > [auto build test WARNING on jack-fs/fsnotify]
> > > [also build test WARNING on pcmoore-audit/next linus/master v5.19 next-20220810]
> > > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > > And when submitting patch, we suggest to use '--base' as documented in
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > >
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
> > > config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220810/202208102231.qSUdYAdb-lkp@intel.com/config)
> > > compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
> > > reproduce (this is a W=1 build):
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # https://github.com/intel-lab-lkp/linux/commit/a943676abc023c094f05b45f4d61936c567507a2
> > >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> > >         git fetch --no-tags linux-review Richard-Guy-Briggs/fanotify-Allow-user-space-to-pass-back-additional-audit-info/20220810-012825
> > >         git checkout a943676abc023c094f05b45f4d61936c567507a2
> > >         # save the config file
> > >         mkdir build_dir && cp config build_dir/.config
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/notify/fanotify/
> > >
> > > If you fix the issue, kindly add following tag where applicable
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>):
> > >
> > > >> fs/notify/fanotify/fanotify_user.c:325:35: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
> >
> > Interesting.  When I "fix" it, my compiler complains:
> >
> >         fs/notify/fanotify/fanotify_user.c:324:11: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 8 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=]
> 
> The correct format specifier for size_t is %zu.  This avoids issues
> between ILP32 vs LP64 targets.

Perfect, thanks!

> > >                     group, fd, response, info_buf, count);
> > >                                                    ^~~~~
> > >    include/linux/printk.h:594:38: note: expanded from macro 'pr_debug'
> > >            no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
> > >                                        ~~~     ^~~~~~~~~~~
> > >    include/linux/printk.h:131:17: note: expanded from macro 'no_printk'
> > >                    printk(fmt, ##__VA_ARGS__);             \
> > >                           ~~~    ^~~~~~~~~~~
> > >    include/linux/printk.h:464:60: note: expanded from macro 'printk'
> > >    #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> > >                                                        ~~~    ^~~~~~~~~~~
> > >    include/linux/printk.h:436:19: note: expanded from macro 'printk_index_wrap'
> > >                    _p_func(_fmt, ##__VA_ARGS__);                           \
> > >                            ~~~~    ^~~~~~~~~~~
> > >    1 warning generated.
> > >
> > >
> > > vim +325 fs/notify/fanotify/fanotify_user.c
> > >
> > >    312
> > >    313        static int process_access_response(struct fsnotify_group *group,
> > >    314                                           struct fanotify_response *response_struct,
> > >    315                                           const char __user *buf,
> > >    316                                           size_t count)
> > >    317        {
> > >    318                struct fanotify_perm_event *event;
> > >    319                int fd = response_struct->fd;
> > >    320                u32 response = response_struct->response;
> > >    321                struct fanotify_response_info_header info_hdr;
> > >    322                char *info_buf = NULL;
> > >    323
> > >    324                pr_debug("%s: group=%p fd=%d response=%u buf=%p size=%lu\n", __func__,
> > >  > 325                         group, fd, response, info_buf, count);
> > >    326                /*
> > >    327                 * make sure the response is valid, if invalid we do nothing and either
> > >    328                 * userspace can send a valid response or we will clean it up after the
> > >    329                 * timeout
> > >    330                 */
> > >    331                if (response & ~FANOTIFY_RESPONSE_VALID_MASK)
> > >    332                        return -EINVAL;
> > >    333                switch (response & FANOTIFY_RESPONSE_ACCESS) {
> > >    334                case FAN_ALLOW:
> > >    335                case FAN_DENY:
> > >    336                        break;
> > >    337                default:
> > >    338                        return -EINVAL;
> > >    339                }
> > >    340                if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
> > >    341                        return -EINVAL;
> > >    342                if (fd < 0)
> > >    343                        return -EINVAL;
> > >    344                if (response & FAN_INFO) {
> > >    345                        size_t c = count;
> > >    346                        const char __user *ib = buf;
> > >    347
> > >    348                        if (c <= 0)
> > >    349                                return -EINVAL;
> > >    350                        while (c >= sizeof(info_hdr)) {
> > >    351                                if (copy_from_user(&info_hdr, ib, sizeof(info_hdr)))
> > >    352                                        return -EFAULT;
> > >    353                                if (info_hdr.pad != 0)
> > >    354                                        return -EINVAL;
> > >    355                                if (c < info_hdr.len)
> > >    356                                        return -EINVAL;
> > >    357                                switch (info_hdr.type) {
> > >    358                                case FAN_RESPONSE_INFO_AUDIT_RULE:
> > >    359                                        break;
> > >    360                                case FAN_RESPONSE_INFO_NONE:
> > >    361                                default:
> > >    362                                        return -EINVAL;
> > >    363                                }
> > >    364                                c -= info_hdr.len;
> > >    365                                ib += info_hdr.len;
> > >    366                        }
> > >    367                        if (c != 0)
> > >    368                                return -EINVAL;
> > >    369                        /* Simplistic check for now */
> > >    370                        if (count != sizeof(struct fanotify_response_info_audit_rule))
> > >    371                                return -EINVAL;
> > >    372                        info_buf = kmalloc(sizeof(struct fanotify_response_info_audit_rule),
> > >    373                                           GFP_KERNEL);
> > >    374                        if (!info_buf)
> > >    375                                return -ENOMEM;
> > >    376                        if (copy_from_user(info_buf, buf, count))
> > >    377                                return -EFAULT;
> > >    378                }
> > >    379                spin_lock(&group->notification_lock);
> > >    380                list_for_each_entry(event, &group->fanotify_data.access_list,
> > >    381                                    fae.fse.list) {
> > >    382                        if (event->fd != fd)
> > >    383                                continue;
> > >    384
> > >    385                        list_del_init(&event->fae.fse.list);
> > >    386                        /* finish_permission_event() eats info_buf */
> > >    387                        finish_permission_event(group, event, response_struct,
> > >    388                                                count, info_buf);
> > >    389                        wake_up(&group->fanotify_data.access_waitq);
> > >    390                        return 0;
> > >    391                }
> > >    392                spin_unlock(&group->notification_lock);
> > >    393
> > >    394                return -ENOENT;
> > >    395        }
> > >    396
> > >
> > > --
> > > 0-DAY CI Kernel Test Service
> > > https://01.org/lkp
> > >
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Sr. S/W Engineer, Kernel Security, Base Operating Systems
> > Remote, Ottawa, Red Hat Canada
> > IRC: rgb, SunRaycer
> > Voice: +1.647.777.2635, Internal: (81) 32635
> >
> >
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-19 21:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <8767f3a0d43d6a994584b86c03eb659a662cc416.1659996830.git.rgb@redhat.com>
2022-08-10 14:28 ` [PATCH v4 2/4] fanotify: define struct members to hold response decision context kernel test robot
2022-08-19 16:25   ` Richard Guy Briggs
2022-08-19 17:17     ` Nick Desaulniers
2022-08-19 21:45       ` Richard Guy Briggs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox