lkml.org 
[lkml]   [2013]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch 06/15] clocksource: Split out user string input
On Thu, Apr 25, 2013 at 1:31 PM, Thomas Gleixner <tglx@linutronix.de> wrote:

> Split out the user string input for clocksource override. Preparatory
> patch for unbind.

This patch has a bug that causes the shell to hang when \n terminated
clocksource strings are sent to the sysfs interface.

echo hpet > /sys/devices/system/clocksource/clocksource0/current_clocksource
<shell hang>


> +static size_t clocksource_get_uname(const char *buf, char *dst, size_t
> cnt)
> +{
> + /* strings from sysfs write are not 0 terminated! */
> + if (!cnt || cnt >= CS_NAME_LEN)
> + return -EINVAL;
> +
> + /* strip of \n: */
> + if (buf[cnt-1] == '\n')
> + cnt--;
> + if (cnt > 0)
> + memcpy(dst, buf, cnt);
> + dst[cnt] = 0;
> + return cnt;
> +}
> +
> /**
> * sysfs_override_clocksource - interface for manually overriding
> clocksource
> * @dev: unused
> @@ -847,22 +863,13 @@ static ssize_t sysfs_override_clocksourc
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - size_t ret = count;
> -
> - /* strings from sysfs write are not 0 terminated! */
> - if (count >= sizeof(override_name))
> - return -EINVAL;
> -
> - /* strip of \n: */
> - if (buf[count-1] == '\n')
> - count--;
> + size_t ret;
>
> mutex_lock(&clocksource_mutex);
>
> - if (count > 0)
> - memcpy(override_name, buf, count);
> - override_name[count] = 0;
> - clocksource_select(false);
> + ret = clocksource_get_uname(buf, override_name, count);
> + if (ret >= 0)
> + clocksource_select(false);
>
> mutex_unlock(&clocksource_mutex);


The change above *looks* ok, but is subtlly different then what we had
before. Note, in the old code, we saved count to ret, and never modified
ret before returning it.

With the new code, we modify cnt, as we process the copied clocksource
name, and return cnt, thus we don't return to the sysfs interface the
same count value we were given, causing the write call to loop.

So you probably want something like the following patch to fix this.

thanks
-john



diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index e9c4f04..5f6c324 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -836,6 +836,8 @@ sysfs_show_current_clocksources(struct device *dev,

static size_t clocksource_get_uname(const char *buf, char *dst, size_t
cnt)
{
+ size_t ret = cnt;
+
/* strings from sysfs write are not 0 terminated! */
if (!cnt || cnt >= CS_NAME_LEN)
return -EINVAL;
@@ -846,7 +848,7 @@ static size_t clocksource_get_uname(const char *buf,
char *dst, size_t cnt)
if (cnt > 0)
memcpy(dst, buf, cnt);
dst[cnt] = 0;
- return cnt;
+ return ret;
}

/**

\
 
 \ /
  Last update: 2013-04-30 01:41    [W:0.358 / U:23.956 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site