| Message ID | 20240821072238.3028-1-shenlichuan@vivo.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v1] security/apparmor: remove duplicate unpacking in unpack_perm function | expand |
On 8/21/24 00:22, Shen Lichuan wrote: > The code was unpacking the 'allow' parameter twice. > This change removes the duplicate part. > > Signed-off-by: Shen Lichuan <shenlichuan@vivo.com> NAK, this would break the unpack. The first entry is actually a reserved value and is just being thrown away atm. Instead of double unpacking to perms->allow we could unpack it to a temp variable that just gets discarded > --- > 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 5a570235427d..4ec1e1251012 100644 > --- a/security/apparmor/policy_unpack.c > +++ b/security/apparmor/policy_unpack.c > @@ -649,7 +649,6 @@ static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm) > return false; > > return aa_unpack_u32(e, &perm->allow, NULL) && > - aa_unpack_u32(e, &perm->allow, NULL) && > aa_unpack_u32(e, &perm->deny, NULL) && > aa_unpack_u32(e, &perm->subtree, NULL) && > aa_unpack_u32(e, &perm->cond, NULL) &&
On Mon, Sep 09, 2024 at 11:57:05PM -0700, John Johansen wrote: > On 8/21/24 00:22, Shen Lichuan wrote: > > The code was unpacking the 'allow' parameter twice. > > This change removes the duplicate part. > > > > Signed-off-by: Shen Lichuan <shenlichuan@vivo.com> > > NAK, this would break the unpack. The first entry is actually a reserved > value and is just being thrown away atm. Instead of double unpacking to > perms->allow we could unpack it to a temp variable that just gets discarded Heh, I recon this should probably be documented in a comment? :) > > > > --- > > 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 5a570235427d..4ec1e1251012 100644 > > --- a/security/apparmor/policy_unpack.c > > +++ b/security/apparmor/policy_unpack.c > > @@ -649,7 +649,6 @@ static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm) > > return false; > > return aa_unpack_u32(e, &perm->allow, NULL) && > > - aa_unpack_u32(e, &perm->allow, NULL) && > > aa_unpack_u32(e, &perm->deny, NULL) && > > aa_unpack_u32(e, &perm->subtree, NULL) && > > aa_unpack_u32(e, &perm->cond, NULL) && >
On 9/10/24 13:57, Serge E. Hallyn wrote: > On Mon, Sep 09, 2024 at 11:57:05PM -0700, John Johansen wrote: >> On 8/21/24 00:22, Shen Lichuan wrote: >>> The code was unpacking the 'allow' parameter twice. >>> This change removes the duplicate part. >>> >>> Signed-off-by: Shen Lichuan <shenlichuan@vivo.com> >> >> NAK, this would break the unpack. The first entry is actually a reserved >> value and is just being thrown away atm. Instead of double unpacking to >> perms->allow we could unpack it to a temp variable that just gets discarded > > Heh, I recon this should probably be documented in a comment? :) yes, definitely. >> >> >>> --- >>> 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 5a570235427d..4ec1e1251012 100644 >>> --- a/security/apparmor/policy_unpack.c >>> +++ b/security/apparmor/policy_unpack.c >>> @@ -649,7 +649,6 @@ static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm) >>> return false; >>> return aa_unpack_u32(e, &perm->allow, NULL) && >>> - aa_unpack_u32(e, &perm->allow, NULL) && >>> aa_unpack_u32(e, &perm->deny, NULL) && >>> aa_unpack_u32(e, &perm->subtree, NULL) && >>> aa_unpack_u32(e, &perm->cond, NULL) && >>
diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 5a570235427d..4ec1e1251012 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -649,7 +649,6 @@ static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm) return false; return aa_unpack_u32(e, &perm->allow, NULL) && - aa_unpack_u32(e, &perm->allow, NULL) && aa_unpack_u32(e, &perm->deny, NULL) && aa_unpack_u32(e, &perm->subtree, NULL) && aa_unpack_u32(e, &perm->cond, NULL) &&
The code was unpacking the 'allow' parameter twice. This change removes the duplicate part. Signed-off-by: Shen Lichuan <shenlichuan@vivo.com> --- security/apparmor/policy_unpack.c | 1 - 1 file changed, 1 deletion(-)