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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 8 Oct 2021 08:31:29 +0100 From: Colin Ian King <colin.king@...onical.com> To: Dan Carpenter <dan.carpenter@...cle.com> Cc: Christian Lamparter <chunkeey@...glemail.com>, Kalle Valo <kvalo@...eaurora.org>, "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, "John W . Linville" <linville@...driver.com>, linux-wireless@...r.kernel.org, netdev@...r.kernel.org, kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] carl9170: Fix error return -EAGAIN if not started On 08/10/2021 06:58, Dan Carpenter wrote: > On Fri, Oct 08, 2021 at 01:15:58AM +0100, Colin King wrote: >> From: Colin Ian King <colin.king@...onical.com> >> >> There is an error return path where the error return is being >> assigned to err rather than count and the error exit path does >> not return -EAGAIN as expected. Fix this by setting the error >> return to variable count as this is the value that is returned >> at the end of the function. >> >> Addresses-Coverity: ("Unused value") >> Fixes: 00c4da27a421 ("carl9170: firmware parser and debugfs code") >> Signed-off-by: Colin Ian King <colin.king@...onical.com> >> --- >> drivers/net/wireless/ath/carl9170/debug.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c >> index bb40889d7c72..f163c6bdac8f 100644 >> --- a/drivers/net/wireless/ath/carl9170/debug.c >> +++ b/drivers/net/wireless/ath/carl9170/debug.c >> @@ -628,7 +628,7 @@ static ssize_t carl9170_debugfs_bug_write(struct ar9170 *ar, const char *buf, >> >> case 'R': >> if (!IS_STARTED(ar)) { >> - err = -EAGAIN; >> + count = -EAGAIN; >> goto out; > > This is ugly. The bug wouldn't have happened with a direct return, it's > only the goto out which causes it. Better to replace all the error > paths with direct returns. There are two other direct returns so it's > not like a new thing... Yep, I agree it was ugly, I was trying to keep to the coding style and reduce the patch delta size. I can do a V2 if the maintainers deem it's a cleaner solution. > > Goto out on the success path is fine here, though. Yep. I believe that a goto to one exit return point (may possibly?) make the code smaller rather than a sprinkling of returns in a function, so I'm never sure if this is a win or not with these kind of cases. Colin > > regards, > dan carpenter >
Powered by blists - more mailing lists