2013-10-11 17:12:15

by Patrick Palka

[permalink] [raw]
Subject: [PATCH] time: Fix signedness bug in sysfs_get_uname() and its callers

sysfs_get_uname() is erroneously declared as returning size_t even
though it may return a negative value, specifically -EINVAL. Its
callers then check whether its return value is less than zero and indeed
that is never the case for size_t.

This patch changes sysfs_get_uname() to return ssize_t and makes sure
its callers use ssize_t accordingly.

Signed-off-by: Patrick Palka <[email protected]>
---
kernel/time/clockevents.c | 2 +-
kernel/time/clocksource.c | 6 +++---
kernel/time/tick-internal.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 38959c8..30554b9 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -584,7 +584,7 @@ static ssize_t sysfs_unbind_tick_dev(struct device *dev,
const char *buf, size_t count)
{
char name[CS_NAME_LEN];
- size_t ret = sysfs_get_uname(buf, name, count);
+ ssize_t ret = sysfs_get_uname(buf, name, count);
struct clock_event_device *ce;

if (ret < 0)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 50a8736..b286f42 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -893,7 +893,7 @@ sysfs_show_current_clocksources(struct device *dev,
return count;
}

-size_t sysfs_get_uname(const char *buf, char *dst, size_t cnt)
+ssize_t sysfs_get_uname(const char *buf, char *dst, size_t cnt)
{
size_t ret = cnt;

@@ -924,7 +924,7 @@ static ssize_t sysfs_override_clocksource(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
- size_t ret;
+ ssize_t ret;

mutex_lock(&clocksource_mutex);

@@ -952,7 +952,7 @@ static ssize_t sysfs_unbind_clocksource(struct device *dev,
{
struct clocksource *cs;
char name[CS_NAME_LEN];
- size_t ret;
+ ssize_t ret;

ret = sysfs_get_uname(buf, name, count);
if (ret < 0)
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index bc906ca..18e71f7 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -31,7 +31,7 @@ extern void tick_install_replacement(struct clock_event_device *dev);

extern void clockevents_shutdown(struct clock_event_device *dev);

-extern size_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);
+extern ssize_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);

/*
* NO_HZ / high resolution timer shared code
--
1.8.4.rc3


2013-10-19 00:16:06

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH] time: Fix signedness bug in sysfs_get_uname() and its callers

On 10/11/2013 10:11 AM, Patrick Palka wrote:
> sysfs_get_uname() is erroneously declared as returning size_t even
> though it may return a negative value, specifically -EINVAL. Its
> callers then check whether its return value is less than zero and indeed
> that is never the case for size_t.
>
> This patch changes sysfs_get_uname() to return ssize_t and makes sure
> its callers use ssize_t accordingly.

So a similar fix has already been queued in tip/timers/core, but this
seems more complete, so I've resolved the collisions with the earlier
fix and queued it for 3.13.

Would you please take a look at the resulting commit and double check I
didn't flub the conflict resolution?

https://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=891292a767c2453af0e5be9465e95b06b4b29ebe;hp=b7bc50e45111e59419474154736f419a555158d9

thanks
-john

2013-10-19 00:51:55

by Patrick Palka

[permalink] [raw]
Subject: Re: [PATCH] time: Fix signedness bug in sysfs_get_uname() and its callers

On Fri, Oct 18, 2013 at 8:16 PM, John Stultz <[email protected]> wrote:
> On 10/11/2013 10:11 AM, Patrick Palka wrote:
>> sysfs_get_uname() is erroneously declared as returning size_t even
>> though it may return a negative value, specifically -EINVAL. Its
>> callers then check whether its return value is less than zero and indeed
>> that is never the case for size_t.
>>
>> This patch changes sysfs_get_uname() to return ssize_t and makes sure
>> its callers use ssize_t accordingly.
>
> So a similar fix has already been queued in tip/timers/core, but this
> seems more complete, so I've resolved the collisions with the earlier
> fix and queued it for 3.13.
>
> Would you please take a look at the resulting commit and double check I
> didn't flub the conflict resolution?
>
> https://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=commitdiff;h=891292a767c2453af0e5be9465e95b06b4b29ebe;hp=b7bc50e45111e59419474154736f419a555158d9

Looks good.