fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

PATCH In review Fengguang Wu 3 participants 7 messages Sep 21, 2012 Kernel janitors

lore

Share & tools

lore

Fengguang Wu

author

2012-09-21 08:22 UTC #0 lore raw

PATCH 86/150 fs/ncpfs/inode.c:335 ncp_show_options() warn: if();

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));

Quoted 1 line

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

Eric W. Biederman

2012-09-21 08:31 UTC #1 lore raw

Quoted 4 lines · Fengguang Wu

Hi Eric,
FYI, there are new smatch warnings show up in

Quoted 8 lines

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

Quoted 30 lines

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));

Quoted 1 line

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

Fengguang Wu

author

2012-09-21 08:40 UTC #2 lore raw

In reply to Fengguang Wu

Quoted 12 lines · Eric W. Biederman

Quoted 10 lines · 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:

Quoted 7 lines

Quoted 2 lines
+ 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

Dan Carpenter

2012-09-21 08:44 UTC #3 lore raw

In reply to Fengguang Wu

Quoted 16 lines · Fengguang Wu

Quoted 15 lines · Eric W. Biederman
Quoted 12 lines · 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

Quoted 8 lines

Quoted 4 lines
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

Dan Carpenter

2012-09-21 08:51 UTC #4 lore raw

In reply to Fengguang Wu

Quoted 22 lines · Dan Carpenter

Quoted 21 lines · Fengguang Wu
Quoted 19 lines · Eric W. Biederman
Quoted 4 lines · Fengguang Wu
Hi Eric,
FYI, there are new smatch warnings show up in
Quoted 8 lines
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.

Quoted 6 lines

Quoted 3 lines
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

Fengguang Wu

author

2012-09-21 08:53 UTC #5 lore raw

In reply to Fengguang Wu

Quoted 22 lines · Dan Carpenter

Quoted 21 lines · Fengguang Wu
Quoted 19 lines · Eric W. Biederman
Quoted 4 lines · Fengguang Wu
Hi Eric,
FYI, there are new smatch warnings show up in
Quoted 8 lines
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.

Quoted 6 lines

Quoted 3 lines
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 <[email protected]>
AuthorDate: Mon Sep 10 00:39:22 2012 +0300
Commit:     Dan Carpenter <[email protected]>
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

Fengguang Wu

author

2012-09-21 08:55 UTC #6 lore raw

In reply to Fengguang Wu

Quoted 30 lines · Dan Carpenter

Quoted 22 lines · Dan Carpenter
Quoted 21 lines · Fengguang Wu
Quoted 19 lines · Eric W. Biederman
Quoted 4 lines · Fengguang Wu
Hi Eric,
FYI, there are new smatch warnings show up in
Quoted 8 lines
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.
Quoted 6 lines
Quoted 3 lines
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.

Quoted 6 lines

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