| Message ID | 1e37fe86-2e83-c1f0-f43b-69ed2fb5c7aa@users.sourceforge.net (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
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. > } > > 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; > } >
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; >> } >> >
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; } 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; }