Re: [2.6 patch] security/selinux/netlabel.c: fix double free
On Monday 28 January 2008 5:35:40 pm Adrian Bunk wrote: > On Mon, Jan 28, 2008 at 05:23:46PM -0500, Paul Moore wrote: > > Thanks for finding this mistake, however, I'd rather see it fixed > > by removing the netlbl_secattr_destroy() call in > > security_netlbl_sid_to_secattr() as it really shouldn't be there > > anymore. We moved the matching _init() call into > > selinux_netlbl_sock_setsid() and I'd like to see the _init() and > > _destroy() calls done in the same function. I can push a revised > > patch for this if you would prefer, otherwise I'll be happy to ack > > an updated version ... > > doing the patch is trivial but you are able to write a better > changelog for it - just push a revised patch. Will do, thanks. -- paul moore linux security @ hp -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] security/selinux/netlabel.c: fix double free
On Mon, Jan 28, 2008 at 05:23:46PM -0500, Paul Moore wrote: > On Monday 28 January 2008 5:09:38 pm Adrian Bunk wrote: > > This patch fixes a double free (security_netlbl_sid_to_secattr() > > already calls netlbl_secattr_destroy() when it returns !0) introduced > > by commit 45c950e0f839fded922ebc0bfd59b1081cc71b70 and spotted by the > > Coverity checker. > > Hi Adrian, Hi Paul, > Thanks for finding this mistake, however, I'd rather see it fixed by > removing the netlbl_secattr_destroy() call in > security_netlbl_sid_to_secattr() as it really shouldn't be there > anymore. We moved the matching _init() call into > selinux_netlbl_sock_setsid() and I'd like to see the _init() and > _destroy() calls done in the same function. I can push a revised patch > for this if you would prefer, otherwise I'll be happy to ack an updated > version ... doing the patch is trivial but you are able to write a better changelog for it - just push a revised patch. > paul moore cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] security/selinux/netlabel.c: fix double free
On Monday 28 January 2008 5:09:38 pm Adrian Bunk wrote:
> This patch fixes a double free (security_netlbl_sid_to_secattr()
> already calls netlbl_secattr_destroy() when it returns !0) introduced
> by commit 45c950e0f839fded922ebc0bfd59b1081cc71b70 and spotted by the
> Coverity checker.
Hi Adrian,
Thanks for finding this mistake, however, I'd rather see it fixed by
removing the netlbl_secattr_destroy() call in
security_netlbl_sid_to_secattr() as it really shouldn't be there
anymore. We moved the matching _init() call into
selinux_netlbl_sock_setsid() and I'd like to see the _init() and
_destroy() calls done in the same function. I can push a revised patch
for this if you would prefer, otherwise I'll be happy to ack an updated
version ...
> Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>
>
> ---
> --- linux-2.6/security/selinux/netlabel.c.old 2008-01-23
> 00:38:19.0 +0200 +++
> linux-2.6/security/selinux/netlabel.c 2008-01-23 00:39:09.0
> +0200 @@ -58,22 +58,22 @@ static int selinux_netlbl_sock_setsid(st rc
> = security_netlbl_sid_to_secattr(sid, );
> if (rc != 0)
> goto sock_setsid_return;
> rc = netlbl_sock_setattr(sk, );
> if (rc == 0) {
> spin_lock_bh(>nlbl_lock);
> sksec->nlbl_state = NLBL_LABELED;
> spin_unlock_bh(>nlbl_lock);
> }
>
> -sock_setsid_return:
> netlbl_secattr_destroy();
> +sock_setsid_return:
> return rc;
> }
>
> /**
> * selinux_netlbl_cache_invalidate - Invalidate the NetLabel cache
> *
> * Description:
> * Invalidate the NetLabel security attribute mapping cache.
> *
> */
--
paul moore
linux security @ hp
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] security/selinux/netlabel.c: fix double free
On Monday 28 January 2008 5:09:38 pm Adrian Bunk wrote:
This patch fixes a double free (security_netlbl_sid_to_secattr()
already calls netlbl_secattr_destroy() when it returns !0) introduced
by commit 45c950e0f839fded922ebc0bfd59b1081cc71b70 and spotted by the
Coverity checker.
Hi Adrian,
Thanks for finding this mistake, however, I'd rather see it fixed by
removing the netlbl_secattr_destroy() call in
security_netlbl_sid_to_secattr() as it really shouldn't be there
anymore. We moved the matching _init() call into
selinux_netlbl_sock_setsid() and I'd like to see the _init() and
_destroy() calls done in the same function. I can push a revised patch
for this if you would prefer, otherwise I'll be happy to ack an updated
version ...
Signed-off-by: Adrian Bunk [EMAIL PROTECTED]
---
--- linux-2.6/security/selinux/netlabel.c.old 2008-01-23
00:38:19.0 +0200 +++
linux-2.6/security/selinux/netlabel.c 2008-01-23 00:39:09.0
+0200 @@ -58,22 +58,22 @@ static int selinux_netlbl_sock_setsid(st rc
= security_netlbl_sid_to_secattr(sid, secattr);
if (rc != 0)
goto sock_setsid_return;
rc = netlbl_sock_setattr(sk, secattr);
if (rc == 0) {
spin_lock_bh(sksec-nlbl_lock);
sksec-nlbl_state = NLBL_LABELED;
spin_unlock_bh(sksec-nlbl_lock);
}
-sock_setsid_return:
netlbl_secattr_destroy(secattr);
+sock_setsid_return:
return rc;
}
/**
* selinux_netlbl_cache_invalidate - Invalidate the NetLabel cache
*
* Description:
* Invalidate the NetLabel security attribute mapping cache.
*
*/
--
paul moore
linux security @ hp
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] security/selinux/netlabel.c: fix double free
On Mon, Jan 28, 2008 at 05:23:46PM -0500, Paul Moore wrote: On Monday 28 January 2008 5:09:38 pm Adrian Bunk wrote: This patch fixes a double free (security_netlbl_sid_to_secattr() already calls netlbl_secattr_destroy() when it returns !0) introduced by commit 45c950e0f839fded922ebc0bfd59b1081cc71b70 and spotted by the Coverity checker. Hi Adrian, Hi Paul, Thanks for finding this mistake, however, I'd rather see it fixed by removing the netlbl_secattr_destroy() call in security_netlbl_sid_to_secattr() as it really shouldn't be there anymore. We moved the matching _init() call into selinux_netlbl_sock_setsid() and I'd like to see the _init() and _destroy() calls done in the same function. I can push a revised patch for this if you would prefer, otherwise I'll be happy to ack an updated version ... doing the patch is trivial but you are able to write a better changelog for it - just push a revised patch. paul moore cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] security/selinux/netlabel.c: fix double free
On Monday 28 January 2008 5:35:40 pm Adrian Bunk wrote: On Mon, Jan 28, 2008 at 05:23:46PM -0500, Paul Moore wrote: Thanks for finding this mistake, however, I'd rather see it fixed by removing the netlbl_secattr_destroy() call in security_netlbl_sid_to_secattr() as it really shouldn't be there anymore. We moved the matching _init() call into selinux_netlbl_sock_setsid() and I'd like to see the _init() and _destroy() calls done in the same function. I can push a revised patch for this if you would prefer, otherwise I'll be happy to ack an updated version ... doing the patch is trivial but you are able to write a better changelog for it - just push a revised patch. Will do, thanks. -- paul moore linux security @ hp -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
