linux-kernel - Re: [PATCH 1/1] kernel-audit: Deletion of an unnecessary check before the function call "audit_log

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:	Sun, 16 Nov 2014 14:24:57 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	SF Markus Elfring <elfring@...rs.sourceforge.net>
Cc:	Eric Paris <eparis@...hat.com>, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org, trivial@...nel.org,
	Coccinelle <cocci@...teme.lip6.fr>
Subject: Re: [PATCH 1/1] kernel-audit: Deletion of an unnecessary check
 before the function call "audit_log_end"

On Sun, Nov 16, 2014 at 11:40:26AM +0100, SF Markus Elfring wrote:
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 21eae3c..1fed61c 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1470,8 +1470,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
>  
>  	/* Send end of event record to help user space know we are finished */
>  	ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
> -	if (ab)
> -		audit_log_end(ab);
> +	audit_log_end(ab);
>  	if (call_panic)
>  		audit_panic("error converting sid to string");
>  }

I should have tried to explain this in my earlier message...

The original code is very clear, the new code works exactly the same but
it's not clear if the author forgot about handling errors from
audit_log_start().  So now someone will come along later and add:

	if (!ab)
		return;

We get a lot of mindless "add error handling" patches like that.  Even
if no one adds that patch who ever is reading the code will think that
the error handling is missing by mistake and have to read the git log
to determine the original intention.

Instead of hiding the readable code in the git log, let's just leave it
in the source file.

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/