| Message ID | 20180823140917.3060915-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | apparmor: remove unused label | expand |
On Thu, Aug 23, 2018 at 7:09 AM, Arnd Bergmann <arnd@arndb.de> wrote: > After the corresponding 'goto' was removed, we get a warning > for the 'fail' label: > > security/apparmor/policy_unpack.c: In function 'unpack_dfa': > security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label] > > Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > security/apparmor/policy_unpack.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c > index 3647b5834ace..96d8cf68ce65 100644 > --- a/security/apparmor/policy_unpack.c > +++ b/security/apparmor/policy_unpack.c > @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e) > > return dfa; > > -fail: > aa_put_dfa(dfa); > return ERR_PTR(-EPROTO); > } > -- > 2.18.0 > -- Kees Cook Pixel Security
On 2018/08/23 23:21, Kees Cook wrote: > On Thu, Aug 23, 2018 at 7:09 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> After the corresponding 'goto' was removed, we get a warning >> for the 'fail' label: >> >> security/apparmor/policy_unpack.c: In function 'unpack_dfa': >> security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label] >> >> Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack") >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Reviewed-by: Kees Cook <keescook@chromium.org> > > -Kees > >> --- >> security/apparmor/policy_unpack.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c >> index 3647b5834ace..96d8cf68ce65 100644 >> --- a/security/apparmor/policy_unpack.c >> +++ b/security/apparmor/policy_unpack.c >> @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e) >> >> return dfa; >> >> -fail: >> aa_put_dfa(dfa); >> return ERR_PTR(-EPROTO); If these lines are unreachable, please remove together... And that is what Gustavo A. R. Silva reported before this patch? >> } >> -- >> 2.18.0 >> > > >
Arnd Bergmann <arnd@arndb.de> writes: > After the corresponding 'goto' was removed, we get a warning > for the 'fail' label: > > security/apparmor/policy_unpack.c: In function 'unpack_dfa': > security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label] > > Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > security/apparmor/policy_unpack.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c > index 3647b5834ace..96d8cf68ce65 100644 > --- a/security/apparmor/policy_unpack.c > +++ b/security/apparmor/policy_unpack.c > @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e) > > return dfa; > > -fail: > aa_put_dfa(dfa); > return ERR_PTR(-EPROTO); > } Shouldn't the two lines after the label be removed as well? IIUC they're unreachable now. -- Thiago Jung Bauermann IBM Linux Technology Center
On 08/23/2018 07:09 AM, Arnd Bergmann wrote: thank you for the patch, but a fix for this issue was pushed to apparmor-next yesterday > After the corresponding 'goto' was removed, we get a warning > for the 'fail' label: > > security/apparmor/policy_unpack.c: In function 'unpack_dfa': > security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label] > > Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > security/apparmor/policy_unpack.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c > index 3647b5834ace..96d8cf68ce65 100644 > --- a/security/apparmor/policy_unpack.c > +++ b/security/apparmor/policy_unpack.c > @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e) > > return dfa; > > -fail: > aa_put_dfa(dfa); > return ERR_PTR(-EPROTO); > } >
On Thu, Aug 23, 2018 at 8:21 PM John Johansen <john.johansen@canonical.com> wrote: > > On 08/23/2018 07:09 AM, Arnd Bergmann wrote: > > thank you for the patch, but a fix for this issue was pushed to apparmor-next yesterday > Ok, good. As several people pointed out, my patch was also wrong, so that saves me doing another one ;-) Arnd
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 3647b5834ace..96d8cf68ce65 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -423,7 +423,6 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e) return dfa; -fail: aa_put_dfa(dfa); return ERR_PTR(-EPROTO); }
After the corresponding 'goto' was removed, we get a warning for the 'fail' label: security/apparmor/policy_unpack.c: In function 'unpack_dfa': security/apparmor/policy_unpack.c:426:1: error: label 'fail' defined but not used [-Werror=unused-label] Fixes: fb5841091f28 ("apparmor: remove no-op permission check in policy_unpack") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- security/apparmor/policy_unpack.c | 1 - 1 file changed, 1 deletion(-) -- 2.18.0