by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v6 05/10] proc: add helpers to set and get proc hidepid and gid mount options

Hi Alexey,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on lwn/docs-next linus/master v5.5-rc3 next-20191220]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Alexey-Gladkov/proc-modernize-proc-to-support-multiple-private-instances/20191226-060818
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1522d9da40bdfe502c91163e6d769332897201fa
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

ld: init/do_mounts.o: in function `proc_fs_pid_gid':
>> do_mounts.c:(.text+0x5): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: arch/x86/kernel/setup.o: in function `proc_fs_pid_gid':
setup.c:(.text+0x3): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: arch/x86/kernel/e820.o: in function `proc_fs_pid_gid':
e820.c:(.text+0xb1): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: arch/x86/kernel/fpu/xstate.o: in function `proc_fs_pid_gid':
xstate.c:(.text+0x36): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: arch/x86/kernel/reboot.o: in function `proc_fs_pid_gid':
reboot.c:(.text+0x1): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: arch/x86/mm/init_32.o: in function `proc_fs_pid_gid':
init_32.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: arch/x86/mm/fault.o: in function `proc_fs_pid_gid':
fault.c:(.text+0x908): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: arch/x86/mm/ioremap.o: in function `proc_fs_pid_gid':
ioremap.c:(.text+0x277): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/fork.o: in function `proc_fs_pid_gid':
fork.c:(.text+0x539): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/exec_domain.o: in function `proc_fs_pid_gid':
exec_domain.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/cpu.o: in function `proc_fs_pid_gid':
cpu.c:(.text+0x104): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/exit.o: in function `proc_fs_pid_gid':
exit.c:(.text+0x22c): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/resource.o: in function `proc_fs_pid_gid':
resource.c:(.text+0x362): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sysctl.o: in function `proc_fs_pid_gid':
sysctl.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/signal.o: in function `proc_fs_pid_gid':
signal.c:(.text+0x55b): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/core.o: in function `proc_fs_pid_gid':
core.c:(.text+0x2e4): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/loadavg.o: in function `proc_fs_pid_gid':
loadavg.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/clock.o: in function `proc_fs_pid_gid':
clock.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/cputime.o: in function `proc_fs_pid_gid':
cputime.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/idle.o: in function `proc_fs_pid_gid':
idle.c:(.text+0x2c): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/fair.o: in function `proc_fs_pid_gid':
fair.c:(.text+0x8cb): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/rt.o: in function `proc_fs_pid_gid':
rt.c:(.text+0x703): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/deadline.o: in function `proc_fs_pid_gid':
deadline.c:(.text+0xb02): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/wait.o: in function `proc_fs_pid_gid':
wait.c:(.text+0x15c): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/wait_bit.o: in function `proc_fs_pid_gid':
wait_bit.c:(.text+0x9d): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/swait.o: in function `proc_fs_pid_gid':
swait.c:(.text+0x4): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/sched/completion.o: in function `proc_fs_pid_gid':
completion.c:(.text+0x4): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/time/timer_list.o: in function `proc_fs_pid_gid':
timer_list.c:(.text+0x12): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: kernel/dma.o: in function `proc_fs_pid_gid':
dma.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: mm/vmstat.o: in function `proc_fs_pid_gid':
vmstat.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: mm/slab_common.o: in function `proc_fs_pid_gid':
slab_common.c:(.text+0x0): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: mm/vmalloc.o: in function `proc_fs_pid_gid':
vmalloc.c:(.text+0x4fd): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: fs/filesystems.o: in function `proc_fs_pid_gid':
filesystems.c:(.text+0x36): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
ld: drivers/char/misc.o: in function `proc_fs_pid_gid':
misc.c:(.text+0xc4): multiple definition of `proc_fs_pid_gid'; init/main.o:main.c:(.text+0x19): first defined here
--
In file included from init/main.c:18:0:
>> include/linux/proc_fs.h:138:47: warning: 'struct proc_info_fs' declared inside parameter list will not be visible outside of this definition or declaration
static inline void proc_fs_set_pid_gid(struct proc_info_fs *fs_info, kgid_t gid)
^~~~~~~~~~~~

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Attachments:

(No filename) (7.51 kB)
.config.gz (7.11 kB)
Download all attachments

2019-12-25 23:10:27

On Mon, Jan 06, 2020 at 06:15:14PM +0300, Alexey Dobriyan wrote:
> > hidepid= Set /proc/<pid>/ access mode.
> > gid= Set the group authorized to learn processes information.
> > + pidonly= Show only task related subset of procfs.
>
> I'd rather have
>
> mount -t proc -o set=pid

This is a great idea.

> so that is can be naturally extended to
>
> mount -t proc -o set=pid,sysctl,misc
>
> > +static int proc_dir_open(struct inode *inode, struct file *file)
> > +{
> > + struct proc_fs_info *fs_info = proc_sb_info(inode->i_sb);
> > +
> > + if (proc_fs_pidonly(fs_info) == PROC_PIDONLY_ON)
> > + return -ENOENT;
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * These are the generic /proc directory operations. They
> > * use the in-memory "struct proc_dir_entry" tree to parse
> > @@ -338,6 +357,7 @@ static const struct file_operations proc_dir_operations = {
> > .llseek = generic_file_llseek,
> > .read = generic_read_dir,
> > .iterate_shared = proc_readdir,
> > + .open = proc_dir_open,
>
> This should not be necessary: if lookup and readdir filters work
> then ->open can't happen.

Yes you are right.

> > --- a/include/linux/proc_fs.h
> > +++ b/include/linux/proc_fs.h
> > +/* definitions for hide_pid field */
> > +enum {
> > + HIDEPID_OFF = 0,
> > + HIDEPID_NO_ACCESS = 1,
> > + HIDEPID_INVISIBLE = 2,
> > + HIDEPID_NOT_PTRACABLE = 3, /* Limit pids to only ptracable pids */
> > +};
>
> These should live in uapi/ as they _are_ user interface to mount().

OK.

What do you think, maybe it's better to make these values a mask ?

I mean:

#define HIDEPID_OFF 0
#define HIDEPID_NO_ACCESS 1
#define HIDEPID_INVISIBLE 2
#define HIDEPID_NOT_PTRACABLE 4

In this case, if in the future there appear values that can be combined,
then there will be no need to make additional parameters.

--
Rgrds, legion