Re: [PATCH 21/46] selinux: Two function calls less in range_read() after error detection

Paul Moore
2017-03-23T22:15:47Z

On Tue, Jan 17, 2017 at 11:35 AM, Casey Schaufler
<casey@schaufler-ca.com> wrote:
> On 1/15/2017 7:21 AM, SF Markus Elfring wrote:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Sat, 14 Jan 2017 20:20:15 +0100
>>
>> Adjust a jump target to avoid two calls of the function "kfree" at the end
>> after a memory allocation failed for the local variable "rt".
>>
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> ---
>> security/selinux/ss/policydb.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
>> index 4cd96ce51322..0d2f64558c0a 100644
>> --- a/security/selinux/ss/policydb.c
>> +++ b/security/selinux/ss/policydb.c
>> @@ -1857,7 +1857,7 @@ static int range_read(struct policydb *p, void *fp)
>> rt = kzalloc(sizeof(*rt), GFP_KERNEL);
>> if (!rt) {
>> rc = -ENOMEM;
>> - goto out;
>> + goto exit;
>
> Why not "return rc;"?
> goto to a return is wrong.

Agree with Casey, but also see my previous comments about the
convenience of using a single error handling goto for loops like this.

>> }
>>
>> rc = next_entry(buf, fp, (sizeof(u32) * 2));
>> @@ -1909,6 +1909,7 @@ static int range_read(struct policydb *p, void *fp)
>> out:
>> kfree(rt);
>> kfree(r);
>> +exit:
>> return rc;
>> }
>>
>

--
paul moore