linux-kernel - Re: [PATCH 4/4] pinctrl: re-enable old state in case of error in pinctrl_select

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, 27 Mar 2013 17:55:45 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Richard Genoud <richard.genoud@...il.com>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	Stephen Warren <swarren@...dia.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] pinctrl: re-enable old state in case of error in
 pinctrl_select_state

On 03/25/2013 08:47 AM, Richard Genoud wrote:
> If a new state is applied, the groups configured in the old state but
> not in the new state are disabled.
> If something goes wrong and the new state can't be applied, we have to
> re-enable those groups.

What is the use-case for this? I wonder if it isn't better to simply
undo the partial selection of the new state (as patch 3/4 attempts to
do) and then leave p->state==NULL, indicating that no state is actively
selected. IIRC, this would be the same as right after the initial
pinctrl_select().

I wonder if it's likely that attempting to re-apply the old state would
actually work, given that applying something just failed.

Finally, this recovery code doesn't:

a) Process anything except MUX_GROUP; any pin config settings in the old
state aren't restored.

b) (I think) Apply any mux settings that don't involve groups that are
referenced by both the old and new states; given that patch 3/4 attempts
to undo everything in the failed application of the new state, I think
this "re-apply the old state" code should simple run through everything
in the old state any apply it unconditionally.

c) Set p->state = oldstate, so it's left at NULL, which would confuse
any future pinctrl_select().
--
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/