linux-kernel - Re: [PATCH] ftrace: return error instead of 12 bytes read

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]

Date:	Wed, 18 Nov 2009 05:18:53 -0500
From:	Dan Merillat <dan.merillat@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Andy Whitcroft <apw@...onical.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Roel Kluin <roel.kluin@...il.com>,
	LKML <linux-kernel@...r.kernel.org>, rostedt@...dmis.org,
	Mike Miller <mike.miller@...com>,
	Jens Axboe <jens.axboe@...cle.com>,
	"Stephen M. Cameron" <scameron@...rdog.cce.hp.com>,
	iss_storagedev@...com
Subject: Re: [PATCH] ftrace: return error instead of 12 bytes read

On Thu, Nov 12, 2009 at 8:45 AM, Ingo Molnar <mingo@...e.hu> wrote:

> Even in filesystems, ~80% of the cases use proper negative values:
>
>  $ git grep 'return -E' fs/ | wc -l
>  4540
>  $ git grep 'return E' fs/ | wc -l
>  895

Except....
fs/9p/fid.c:                            return ERR_PTR(-EPERM);

try this:

$ git grep "return E[A-Z]*;" | grep -v EOF | grep -v ERROR | wc -l
138
$ git grep "return -E[A-Z]*;"| wc -l
57285

2 of those are in Documentation/ and 2 are comments from a quick
glance.   134 uses of positive error returns, _74_ of which are in
fs/xfs, 24 in bluetooth, and the rest scattered randomly around the
kernel.

134 positive error returns  vs 57881 negative?  The style is so
strongly ingrained in the kernel (And I'd bet an audit of those
remaning 134 would find at least one bug) that it'd be a good janitor
task to go through and switch them.


This is one example - a function that returns either ENXIO or 0, and
the lone caller explicitly tests and flips the sign.

(Not signing off on this!!!!  but CCing the relevant people for this driver)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 6399e50..79867d6 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -428,7 +428,7 @@ cciss_proc_write(struct file *file, const char __user *buf,

                rc = cciss_engage_scsi(h->ctlr);
                if (rc != 0)
-                       err = -rc;
+                       err = rc;
                else
                        err = length;
        } else
diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c
index 3315268..4af3085 100644
--- a/drivers/block/cciss_scsi.c
+++ b/drivers/block/cciss_scsi.c
@@ -1547,7 +1547,7 @@ cciss_engage_scsi(int ctlr)
        if (sa->registered) {
                printk("cciss%d: SCSI subsystem already engaged.\n", ctlr);
                spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
-               return ENXIO;
+               return -ENXIO;
        }
        sa->registered = 1;
        spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
--
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/