linux-kernel - Re: [PATCH 1/2] User access to internal clocks

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: <20090209181219.GC19453@n2100.arm.linux.org.uk>
Date:	Mon, 9 Feb 2009 18:12:19 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Davide Rizzo <elpa.rizzo@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
	linux-arm-kernel@...ts.arm.linux.org.uk, ben-linux@...ff.org,
	Greg KH <gregkh@...e.de>
Subject: Re: [PATCH 1/2] User access to internal clocks

On Mon, Feb 09, 2009 at 06:04:29PM +0100, Davide Rizzo wrote:
> +static ssize_t parent_store(struct device *dev,
> +       struct device_attribute *attr, const char *buffer, size_t count)
> +{
> +       struct clk *parent;
> +
> +       char *s = kstrdup(buffer, GFP_KERNEL);
> +
> +       if (!s)
> +               return -ENOMEM;
> +       if (s[strlen(s) - 1] == '\n')
> +               s[strlen(s) - 1] = '\0';
> +       parent = clk_get(dev, s);
> +       kfree(s);
> +       if (IS_ERR(parent))
> +               return PTR_ERR(parent);
> +
> +       mutex_lock(&mutex);
> +
> +       clk_set_parent(dev_get_drvdata(dev), parent);

BTW, MXC doesn't (and won't) support run-time changing of the clk tree.
OMAP sort-of does, but it's currently buggy.  It looks like S3C is also
buggy.

These implementations are currently only usable under a very strict set
of circumstances (which are not currently checked by the implementations)
and if the parent is changed while the clock is in use, it'll screw up
the clock tree - details below.

Arguably, that's a problem which the clk_set_parent() implementations need
to be solved (and I hope are going to be resolved), but it's an example
where exposing the API to allow userspace to fiddle with things produces
new ways to make things explode.  Certainly the MXC folk responded saying
"we don't support run-time parent changes".

FYI, the problem basically is that the sequence:

1. clk_enable(clk);
2. clk_set_parent(clk, new_parent);
3. clk_disable(clk);

will result in the old parent having a permanently incremented usecount,
and the new_parent having a negative usecount.  You could say "don't
permit enabled clocks to change parents" but I believe such a statement
would be provably incompatible with OMAP.

(This issue has just been pointed out to the relevant ARM maintainers
today.)
--
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/