[userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[Fengguang Wu]

Hi Eric,

FYI, there are new smatch warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git userns-always-map-user-v58
head:   cf7198a6b74906aa363a8507d6f9cbdf56e42dc1
commit: 84ec77ea8a96e568f20bec4465c1fbde2c066cf8 [86/150] userns: Convert ncpfs to use kuid and kgid where appropriate

All smatch warnings:

+ fs/ncpfs/inode.c:335 ncp_show_options() warn: if();
  fs/ncpfs/inode.c:654 ncp_fill_super() Error invalid range 4096 to -1

vim +335 fs/ncpfs/inode.c
   319			flush_work_sync(&server->tx.tq);
   320		else
   321			flush_work_sync(&server->timeout_tq);
   322	}
   323	
   324	static int  ncp_show_options(struct seq_file *seq, struct dentry *root)
   325	{
   326		struct ncp_server *server = NCP_SBP(root->d_sb);
   327		unsigned int tmp;
   328	
   329		if (!uid_eq(server->m.uid, GLOBAL_ROOT_UID))
   330			seq_printf(seq, ",uid=%u",
   331				   from_kuid_munged(&init_user_ns, server->m.uid));
   332		if (!gid_eq(server->m.gid, GLOBAL_ROOT_GID))
   333			seq_printf(seq, ",gid=%u",
   334				   from_kgid_munged(&init_user_ns, server->m.gid));
 > 335		if (!uid_eq(server->m.mounted_uid, GLOBAL_ROOT_UID));
   336			seq_printf(seq, ",owner=%u",
   337				   from_kuid_munged(&init_user_ns, server->m.mounted_uid));
   338		tmp = server->m.file_mode & S_IALLUGO;
   339		if (tmp != NCP_DEFAULT_FILE_MODE)
   340			seq_printf(seq, ",mode=0%o", tmp);
   341		tmp = server->m.dir_mode & S_IALLUGO;
   342		if (tmp != NCP_DEFAULT_DIR_MODE)
   343			seq_printf(seq, ",dirmode=0%o", tmp);

---
0-DAY kernel build testing backend         Open Source Technology Centre
Fengguang Wu, Yuanhan Liu                              Intel Corporation

Re: [userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[Eric W. Biederman]

Fengguang Wu <fengguang.wu@intel.com> writes:

> Hi Eric,
>
> FYI, there are new smatch warnings show up in
>
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git userns-always-map-user-v58
> head:   cf7198a6b74906aa363a8507d6f9cbdf56e42dc1
> commit: 84ec77ea8a96e568f20bec4465c1fbde2c066cf8 [86/150] userns: Convert ncpfs to use kuid and kgid where appropriate
>
> All smatch warnings:
>
> + fs/ncpfs/inode.c:335 ncp_show_options() warn: if();
>   fs/ncpfs/inode.c:654 ncp_fill_super() Error invalid range 4096 to -1

Good catch.  Thanks.

Did you just start running smatch tests?  I haven't looked at ncpfs for
a long time.

Eric



> vim +335 fs/ncpfs/inode.c
>    319			flush_work_sync(&server->tx.tq);
>    320		else
>    321			flush_work_sync(&server->timeout_tq);
>    322	}
>    323	
>    324	static int  ncp_show_options(struct seq_file *seq, struct dentry *root)
>    325	{
>    326		struct ncp_server *server = NCP_SBP(root->d_sb);
>    327		unsigned int tmp;
>    328	
>    329		if (!uid_eq(server->m.uid, GLOBAL_ROOT_UID))
>    330			seq_printf(seq, ",uid=%u",
>    331				   from_kuid_munged(&init_user_ns, server->m.uid));
>    332		if (!gid_eq(server->m.gid, GLOBAL_ROOT_GID))
>    333			seq_printf(seq, ",gid=%u",
>    334				   from_kgid_munged(&init_user_ns, server->m.gid));
>  > 335		if (!uid_eq(server->m.mounted_uid, GLOBAL_ROOT_UID));
>    336			seq_printf(seq, ",owner=%u",
>    337				   from_kuid_munged(&init_user_ns, server->m.mounted_uid));
>    338		tmp = server->m.file_mode & S_IALLUGO;
>    339		if (tmp != NCP_DEFAULT_FILE_MODE)
>    340			seq_printf(seq, ",mode=0%o", tmp);
>    341		tmp = server->m.dir_mode & S_IALLUGO;
>    342		if (tmp != NCP_DEFAULT_DIR_MODE)
>    343			seq_printf(seq, ",dirmode=0%o", tmp);
>
> ---
> 0-DAY kernel build testing backend         Open Source Technology Centre
> Fengguang Wu, Yuanhan Liu                              Intel Corporation

Re: [userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[Fengguang Wu]

On Fri, Sep 21, 2012 at 01:31:25AM -0700, Eric W. Biederman wrote:
> Fengguang Wu <fengguang.wu@intel.com> writes:
> 
> > Hi Eric,
> >
> > FYI, there are new smatch warnings show up in
> >
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git userns-always-map-user-v58
> > head:   cf7198a6b74906aa363a8507d6f9cbdf56e42dc1
> > commit: 84ec77ea8a96e568f20bec4465c1fbde2c066cf8 [86/150] userns: Convert ncpfs to use kuid and kgid where appropriate
> >
> > All smatch warnings:
> >
> > + fs/ncpfs/inode.c:335 ncp_show_options() warn: if();
> >   fs/ncpfs/inode.c:654 ncp_fill_super() Error invalid range 4096 to -1
> 
> Good catch.  Thanks.
> 
> Did you just start running smatch tests?  I haven't looked at ncpfs for
> a long time.

Yes. :)  I don't quite get the old "invalid range 4096 to -1" warning,
perhaps it's fine to just ignore it.

Thanks,
Fengguang

Re: [userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[Dan Carpenter]

On Fri, Sep 21, 2012 at 04:40:05PM +0800, Fengguang Wu wrote:
> On Fri, Sep 21, 2012 at 01:31:25AM -0700, Eric W. Biederman wrote:
> > Fengguang Wu <fengguang.wu@intel.com> writes:
> > 
> > > Hi Eric,
> > >
> > > FYI, there are new smatch warnings show up in
> > >
> > > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git userns-always-map-user-v58
> > > head:   cf7198a6b74906aa363a8507d6f9cbdf56e42dc1
> > > commit: 84ec77ea8a96e568f20bec4465c1fbde2c066cf8 [86/150] userns: Convert ncpfs to use kuid and kgid where appropriate
> > >
> > > All smatch warnings:
> > >
> > > + fs/ncpfs/inode.c:335 ncp_show_options() warn: if();
> > >   fs/ncpfs/inode.c:654 ncp_fill_super() Error invalid range 4096 to -1
> > 
> > Good catch.  Thanks.
> > 
> > Did you just start running smatch tests?  I haven't looked at ncpfs for
> > a long time.
> 
> Yes. :)  I don't quite get the old "invalid range 4096 to -1" warning,
> perhaps it's fine to just ignore it.
> 

Yes, yes.  This shouldn't be there in the latest Smatch.  (I thought
you were on that thread).  Let me know if it's still an issue.

regards,
dan carpenter

Re: [userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[Dan Carpenter]

On Fri, Sep 21, 2012 at 11:44:58AM +0300, Dan Carpenter wrote:
> On Fri, Sep 21, 2012 at 04:40:05PM +0800, Fengguang Wu wrote:
> > On Fri, Sep 21, 2012 at 01:31:25AM -0700, Eric W. Biederman wrote:
> > > Fengguang Wu <fengguang.wu@intel.com> writes:
> > > 
> > > > Hi Eric,
> > > >
> > > > FYI, there are new smatch warnings show up in
> > > >
> > > > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git userns-always-map-user-v58
> > > > head:   cf7198a6b74906aa363a8507d6f9cbdf56e42dc1
> > > > commit: 84ec77ea8a96e568f20bec4465c1fbde2c066cf8 [86/150] userns: Convert ncpfs to use kuid and kgid where appropriate
> > > >
> > > > All smatch warnings:
> > > >
> > > > + fs/ncpfs/inode.c:335 ncp_show_options() warn: if();
> > > >   fs/ncpfs/inode.c:654 ncp_fill_super() Error invalid range 4096 to -1
> > > 
> > > Good catch.  Thanks.
> > > 
> > > Did you just start running smatch tests?  I haven't looked at ncpfs for
> > > a long time.
> > 
> > Yes. :)  I don't quite get the old "invalid range 4096 to -1" warning,
> > perhaps it's fine to just ignore it.
> > 
> 
> Yes, yes.  This shouldn't be there in the latest Smatch.  (I thought
> you were on that thread).  Let me know if it's still an issue.

So I haven't totally puplicized this but Smatch has a --spammy
option.  If you want something disabled, I could easily move it
under the --spammy option so it doesn't show up by default.

Just add and if (!option_spammy) return in front of the print
statement.  I'm happy to merge all those patches.

regards,
dan carpenter

Re: [userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[Fengguang Wu]

On Fri, Sep 21, 2012 at 11:44:58AM +0300, Dan Carpenter wrote:
> On Fri, Sep 21, 2012 at 04:40:05PM +0800, Fengguang Wu wrote:
> > On Fri, Sep 21, 2012 at 01:31:25AM -0700, Eric W. Biederman wrote:
> > > Fengguang Wu <fengguang.wu@intel.com> writes:
> > > 
> > > > Hi Eric,
> > > >
> > > > FYI, there are new smatch warnings show up in
> > > >
> > > > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git userns-always-map-user-v58
> > > > head:   cf7198a6b74906aa363a8507d6f9cbdf56e42dc1
> > > > commit: 84ec77ea8a96e568f20bec4465c1fbde2c066cf8 [86/150] userns: Convert ncpfs to use kuid and kgid where appropriate
> > > >
> > > > All smatch warnings:
> > > >
> > > > + fs/ncpfs/inode.c:335 ncp_show_options() warn: if();
> > > >   fs/ncpfs/inode.c:654 ncp_fill_super() Error invalid range 4096 to -1
> > > 
> > > Good catch.  Thanks.
> > > 
> > > Did you just start running smatch tests?  I haven't looked at ncpfs for
> > > a long time.
> > 
> > Yes. :)  I don't quite get the old "invalid range 4096 to -1" warning,
> > perhaps it's fine to just ignore it.
> > 
> 
> Yes, yes.  This shouldn't be there in the latest Smatch.  (I thought
> you were on that thread).  Let me know if it's still an issue.

The fix you suggested last time seem to be this, which I pulled and
installed promptly:

commit 72174b5c4bf53b830d71ae04d042722cdab3e1a2
Author:     Dan Carpenter <dan.carpenter@oracle.com>
AuthorDate: Mon Sep 10 00:39:22 2012 +0300
Commit:     Dan Carpenter <dan.carpenter@oracle.com>
CommitDate: Mon Sep 10 00:39:22 2012 +0300

    check_kernel: fix ERR_PTR() false positives

    This was off and it was causing false positives like:
    net/mac80211/key.c:368 ieee80211_key_alloc() error: -4096 too low for ERR_PTR

Anyway I refreshed the smatch tree just now :)

Thanks,
Fengguang

Re: [userns:userns-always-map-user-v58 86/150] fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

[Fengguang Wu]

On Fri, Sep 21, 2012 at 11:51:22AM +0300, Dan Carpenter wrote:
> On Fri, Sep 21, 2012 at 11:44:58AM +0300, Dan Carpenter wrote:
> > On Fri, Sep 21, 2012 at 04:40:05PM +0800, Fengguang Wu wrote:
> > > On Fri, Sep 21, 2012 at 01:31:25AM -0700, Eric W. Biederman wrote:
> > > > Fengguang Wu <fengguang.wu@intel.com> writes:
> > > > 
> > > > > Hi Eric,
> > > > >
> > > > > FYI, there are new smatch warnings show up in
> > > > >
> > > > > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git userns-always-map-user-v58
> > > > > head:   cf7198a6b74906aa363a8507d6f9cbdf56e42dc1
> > > > > commit: 84ec77ea8a96e568f20bec4465c1fbde2c066cf8 [86/150] userns: Convert ncpfs to use kuid and kgid where appropriate
> > > > >
> > > > > All smatch warnings:
> > > > >
> > > > > + fs/ncpfs/inode.c:335 ncp_show_options() warn: if();
> > > > >   fs/ncpfs/inode.c:654 ncp_fill_super() Error invalid range 4096 to -1
> > > > 
> > > > Good catch.  Thanks.
> > > > 
> > > > Did you just start running smatch tests?  I haven't looked at ncpfs for
> > > > a long time.
> > > 
> > > Yes. :)  I don't quite get the old "invalid range 4096 to -1" warning,
> > > perhaps it's fine to just ignore it.
> > > 
> > 
> > Yes, yes.  This shouldn't be there in the latest Smatch.  (I thought
> > you were on that thread).  Let me know if it's still an issue.
> 
> So I haven't totally puplicized this but Smatch has a --spammy
> option.  If you want something disabled, I could easily move it
> under the --spammy option so it doesn't show up by default.
> 
> Just add and if (!option_spammy) return in front of the print
> statement.  I'm happy to merge all those patches.

OK, thanks!

Fengguang