linux-kernel - Re: [PATCH] gpiolib: check if gpio

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: <CAAVeFuLJOSikb3pXffbC7Y4LDKXd-PQ_cadtT1q=TLCyWVh=MA@mail.gmail.com>
Date:	Wed, 19 Mar 2014 10:48:45 +0900
From:	Alexandre Courbot <gnurou@...il.com>
To:	Ben Dooks <ben.dooks@...ethink.co.uk>
Cc:	linux-kernel@...ts.codethink.co.uk,
	Linus Walleij <linus.walleij@...aro.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] gpiolib: check if gpio_desc pointer is error or NULL

On Tue, Mar 18, 2014 at 7:41 PM, Ben Dooks <ben.dooks@...ethink.co.uk> wrote:
> Some of the gpiod_ calls take a pointer to a gpio_desc as their
> argument but only check to see if it is NULL to validate the
> input.
>
> Calls such as devm_gpiod_get() return an error-pointer if they
> fail, so doing the following will not work:
>
>         gpio = devm_gpiod_get(...);
>         gpiod_direction_output(gpio, 0);
>
> The sequence produces an OOPS like:
>
>         Unable to handle kernel paging request at virtual address fffffffe
>
> Change all calls that check for !desc to use IS_ERR_OR_NULL() to
> avoid these issues.

This change is certainly correct from a semantics point of view. Maybe
I'd argue that the burden is on the caller to check that gpiod_get()
returns a valid GPIO descriptor, but having extra security doesn't
hurt. However my problem with this change in its current form is that
it will hide such forgetfulnesses by making functions like
gpiod_get_value() fail silently instead of triggering a oops.

Could you make sure that any call of a gpiolib function on a non-valid
descriptor raises at least a message in the console, and while you are
at it harmonize the way these messages are output? Right now we are
using pr_debug(), pr_warn() or WARN_ON() depending on the location. I
would say that using an invalid GPIO descriptor is offending enough
that it should trigger a stack dump at every attempt.
--
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/