linux-kernel - Re: KEYS: trusted: Use common error handling code in trusted

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] [day] [month] [year] [list]

Date:   Sat, 11 Nov 2017 10:37:36 +0100
From:   SF Markus Elfring <elfring@...rs.sourceforge.net>
To:     Julia Lawall <julia.lawall@...6.fr>, keyrings@...r.kernel.org,
        linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org
Cc:     David Howells <dhowells@...hat.com>,
        James Morris <james.l.morris@...cle.com>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: KEYS: trusted: Use common error handling code in trusted_update()

> Safe means that cleanup code should appear once in a cascade at the end
> of the function, to minimize the chance that anything will be overlooked.

I find that the control flow of this function implementation does not fit
to the mentioned ideal so far.


> Moving the ret assignments to the end of the function and adding the
> backward jumps doesn't make the code more understandable.

Is this structure required if you would like to achieve something
in the shown software design direction?


> On the other hand, moving the kzalloc of new_p to the end of the function
> could be helpful,

Why do you think that the movement of this function call can finally work
in the concrete software situation?


> because it reduces the chance that new error handling code,
> if any turns out to be needed, will forget this operation.

Your expectation can be nice.


> By why not just follow standard practice and free the structures in a
> cascade in the inverse of the order in which they are allocated at the end
> of the function?

This is still happening here partly, isn't it?


> There can be a descriptive label for each thing that needs to be freed.

Which identifiers would you find more appropriate in comparison to
my suggestion?

* e_inval
* e_nomem
* free_data
* free_payload

Regards,
Markus