linux-kernel - Re: [PATCH] staging: unisys: use common return path

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 1 Dec 2015 11:00:45 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
Cc:	Benjamin Romer <benjamin.romer@...sys.com>,
	David Kershner <david.kershner@...sys.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	devel@...verdev.osuosl.org, sparmaintainer@...sys.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: unisys: use common return path

Doing One Err style error handling is often a mistake but it's ok here.
Just choose a better label name, though.  "unlock".  I have seen several
bugs caused because people used the label name "err" instead of
"unlock".  Compare these two examples.

	spin_unlock();
err:
	return ret;

Do you see the bug?  Now look at this code:

	spin_unlock();
unlock:
	return ret;

Just from looking at those few lines you can see that the intent was
to unlock but instead it just returns.  In the first example, you need
to scroll to the start of the function and examine the context to see
what "err" was intended to do.  And you're not likely to even be
suspicious of the err label because do nothing labels with ambiguos
label names are very common.

I have seen at least three places where an ambiguously named label was
placed after the spin unlock instead of before, where it was intended.

regards,
dan carpenter

--
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/