linux-kernel - Re: [V9fs-developer] [PATCH 1/8] fs/9p: Deletion of unnecessary checks before the function call "p9_client

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 6 Jan 2015 10:27:19 +0100
From:	Dominique Martinet <dominique.martinet@....fr>
To:	Julia Lawall <julia.lawall@...6.fr>
CC:	SF Markus Elfring <elfring@...rs.sourceforge.net>,
	Latchesar Ionkov <lucho@...kov.net>,
	Eric Van Hensbergen <ericvh@...il.com>,
	<kernel-janitors@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Ron Minnich <rminnich@...dia.gov>,
	<v9fs-developer@...ts.sourceforge.net>,
	Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [V9fs-developer] [PATCH 1/8] fs/9p: Deletion of unnecessary
 checks before the function call "p9_client_clunk"

Dominique Martinet wrote on Tue, Jan 06, 2015 at 09:12:53AM +0100:
> Simple example, without even looking at the multiple possible
> interactions with multiple clients hammering the server, can be taken
> from v9fs_create in fs/9p/vfs_inode.c
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/fs/9p/vfs_inode.c?id=d8282ea05ad119247122de23db7d48ad6098cfa2#n652
> 
> jumping to label error from a failure in p9_client_fcreate (e.g. EPERM
> or something perfectly valid) will, with your patch, call clunk with fid
> == NULL and thus generate a stack trace, while it is a perfectly normal
> code path that should only return to userspace with EPERM.

Actually just seen that this precise example is fixed along with more
similar code paths in subsequents (!) patchs of the set.

It could actually be interesting to see if we could get all such paths
"fixed".
If so, I believe this way will ultimately lead to cleaner code, but I'd
rather see taken all other patches first and keep this one in v9fs-devel
branch for a bit longer if this makes sense... Well, I guess there is a
test window before the merge to linus tree anyway, but given the number
of active 9P users it could be useful.

(All other patches in the set look good to me at second glance, now.
To answer Dan's mail, none of these are bug fix as far as I've seen,
just avoiding unnecessary checks for null or calls+check/warn depending
on whether you apply patch #1 first)

-- 
Dominique Martinet,
CEA
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/