2008-01-29 02:21:24

by Paul Moore

[permalink] [raw]
Subject: [PATCH] SELinux: Fix double free in selinux_netlbl_sock_setsid()

As pointed out by Adrian Bunk, commit 45c950e0f839fded922ebc0bfd59b1081cc71b70
caused a double-free when security_netlbl_sid_to_secattr() fails. This patch
fixes this by removing the netlbl_secattr_destroy() call from that function
since we are already releasing the secattr memory in
selinux_netlbl_sock_setsid().

Signed-off-by: Paul Moore <[email protected]>
---

security/selinux/ss/services.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 4bf715d..3a16aba 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2629,7 +2629,6 @@ int security_netlbl_sid_to_secattr(u32 sid, struct netlbl_lsm_secattr *secattr)

netlbl_sid_to_secattr_failure:
POLICY_RDUNLOCK;
- netlbl_secattr_destroy(secattr);
return rc;
}
#endif /* CONFIG_NETLABEL */


2008-01-29 03:51:19

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] SELinux: Fix double free in selinux_netlbl_sock_setsid()

From: Paul Moore <[email protected]>
Date: Mon, 28 Jan 2008 21:20:26 -0500

> As pointed out by Adrian Bunk, commit 45c950e0f839fded922ebc0bfd59b1081cc71b70
> caused a double-free when security_netlbl_sid_to_secattr() fails. This patch
> fixes this by removing the netlbl_secattr_destroy() call from that function
> since we are already releasing the secattr memory in
> selinux_netlbl_sock_setsid().
>
> Signed-off-by: Paul Moore <[email protected]>

Applied, and I'll queue this up for -stable too.

Please, when mentioning specific commits please also provide
the changelog headline along with the SHA1 hash.

The reason is that when this fix is moved over to another
tree where the SHA1 of the causing change is different people
studying your fix won't be able to find it without more stable
contextual information.

Thanks.

2008-01-29 17:14:29

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] SELinux: Fix double free in selinux_netlbl_sock_setsid()

On Monday 28 January 2008 10:51:24 pm David Miller wrote:
> From: Paul Moore <[email protected]>
> Date: Mon, 28 Jan 2008 21:20:26 -0500
>
> > As pointed out by Adrian Bunk, commit
> > 45c950e0f839fded922ebc0bfd59b1081cc71b70 caused a double-free when
> > security_netlbl_sid_to_secattr() fails. This patch fixes this by
> > removing the netlbl_secattr_destroy() call from that function since we
> > are already releasing the secattr memory in
> > selinux_netlbl_sock_setsid().
> >
> > Signed-off-by: Paul Moore <[email protected]>
>
> Applied, and I'll queue this up for -stable too.

Thanks. Sorry for not catching this in the first place.

> Please, when mentioning specific commits please also provide
> the changelog headline along with the SHA1 hash.
>
> The reason is that when this fix is moved over to another
> tree where the SHA1 of the causing change is different people
> studying your fix won't be able to find it without more stable
> contextual information.

Noted, I'll make sure to include the patch description in the future. I
wasn't aware that the hash took into account anything other than the
individual commit it represented. However, now that I think about it, since
order is so critical it only makes sense to have the hash take into account
at least the previous commit.

--
paul moore
linux security @ hp