linux-kernel - Re: [KJ] (re) audit return code handling for kernel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20060730004850.GA9344@localhost.localdomain>
Date:	Sat, 29 Jul 2006 20:48:50 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Andrew Morton <akpm@...l.org>
Cc:	kernel-janitors@...ts.osdl.org, linux-kernel@...r.kernel.org,
	torvalds@...l.org, marcel@...tman.org, fpavlic@...ibm.com,
	paulus@...ibm.com, bcollins@...ian.org, tony.luck@...el.com
Subject: Re: [KJ] (re) audit return code handling for kernel_thread [1/3]

On Sat, Jul 29, 2006 at 05:03:33PM -0700, Andrew Morton wrote:
> On Sat, 29 Jul 2006 16:15:55 -0400
> Neil Horman <nhorman@...driver.com> wrote:
> 
> > Patch to audit return code checking of kernel_thread.  These fixes correct those
> > callers who fail to check the return code of kernel_thread at all
> > 
> > 
> 
> Various people are running around converting open-coded kernel_thread
> callers over to the kthread API.  Generally that's a good thing, and error
> checking should be incorporated at that time.
> 
> So there's probably not a lot of point in making these changes now - it'd
> be better to work with the various subsystem owners on doing the kthread
> conversion.
> 
> > --- a/arch/s390/mm/cmm.c
> > +++ b/arch/s390/mm/cmm.c
> > @@ -161,7 +161,11 @@ cmm_thread(void *dummy)
> >  static void
> >  cmm_start_thread(void)
> >  {
> > -	kernel_thread(cmm_thread, NULL, 0);
> > +	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
> > +		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
> > +			__FUNCTION__,__LINE__);
> > +		clear_bit(0,&cmm_thread_active);
> > +	}
> >  }
> 
> This is OK as far as it goes.  But really we should propagate any failure
> back up to cmm_init() and fail the whole thing, rather than leaving the
> driver hanging around in a loaded-but-useless state.


Understood, new patch attached, that removes most of the additional failure to
check return code cases.  I've left the cmm_start_thread case and the
rfcomm_init cases as is, because the cmm_start_thread case is called
asynchronously from a work queue, fired from a timer, meaning we cannot
propogate the error to prevent the module from loading, and the rfcomm_init case
does precisely what you ask, in that it detects a failure to start the kernel
thread, and fails the module load if the thread creation fails.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <nhorman@...driver.com>


 arch/s390/mm/cmm.c          |    6 +++++-
 net/bluetooth/rfcomm/core.c |    6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)


--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -161,7 +161,11 @@
 static void
 cmm_start_thread(void)
 {
-	kernel_thread(cmm_thread, NULL, 0);
+	if (kernel_thread(cmm_thread, NULL, 0) < 0) {
+		printk(KERN_WARNING "Could not start kernel thread at %s:%d\n",
+			__FUNCTION__,__LINE__);
+		clear_bit(0,&cmm_thread_active);
+	}
 }
 
 
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2052,11 +2052,15 @@
 /* ---- Initialization ---- */
 static int __init rfcomm_init(void)
 {
+	int ret;
 	l2cap_load();
 
 	hci_register_cb(&rfcomm_cb);
 
-	kernel_thread(rfcomm_run, NULL, CLONE_KERNEL);
+	ret = kernel_thread(rfcomm_run, NULL, CLONE_KERNEL);
+	
+	if (ret < 0)
+		return ret;
 
 	class_create_file(bt_class, &class_attr_rfcomm_dlc);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/