[linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device - Kernel

This is a discussion on [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device - Kernel ; On Tue, 2008-01-29 at 07:51 -0800, H. Peter Anvin wrote: > Yi Yang wrote: > > Current cpuid module will create a char device for every logical cpu, > > when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless ...

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast
Results 21 to 40 of 51

Thread: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device

  1. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    On Tue, 2008-01-29 at 07:51 -0800, H. Peter Anvin wrote:
    > Yi Yang wrote:
    > > Current cpuid module will create a char device for every logical cpu,
    > > when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop,
    > > the root cause is that cpuid module doesn't decide wether a cpuid level
    > > is valid, it just uses an offset to denote cpuid level and take it to
    > > cpuid instruction, cpuid instruction will ignore it and return some data
    > > specific to cpu model, cpuid doesn't an error return value because it is
    > > void type. So cpuid module will execute cpuid continuously and return
    > > data although most of data make no sense.
    > >
    > > This patch tries to add a sysfs interface for cpuid, users can see all the
    > > available cpuid levels, specify a specific level and get cpuid corresponding
    > > to this cpuid level.
    > >
    > > For every logical cpu, this patch will create a cpuid directory under
    > > /sys/devices/system/cpu/cpu*/, there are three entries under cpuid:
    > >
    > > avail_levels cur_level cur_cpuid
    > >
    > > A user can get all the available cpuid levels from avail_levels, he/she can
    > > set one available cpuid level to cur_level, then he/she can get cpuid from
    > > cur_cpuid, cur_cpuid corresponds to cur_level.
    > >
    > > This patch uses sysfs to avoid limitless loop and provide more flexible
    > > interface for cpuid, please consider to merge to -mm tree in order to test.

    >
    > This is broken.
    >
    > Triple broken.
    >
    > It's broken, because it doesn't take into account the fact that Intel
    > broke CPUID level 4 and made it "repeating" (neither did the cpuid char
    > device, because it predated the Intel braindamage; I've had a patch for
    > it privately for a while, but didn't push it upstream because paravirt
    > broke it royally and I wanted the situation to settle down.)
    >
    > It's broken, because the algorithm used to determine valid CPUID levels
    > is incorrect; it fails to recognize any CPUID levels other than the main
    > Intel and AMD ones, e.g. the Transmeta 0x8086xxxx (and sometimes more)
    > and VIA 0xc000xxxx levels.

    Thank you for pointing out these issues, i think we can let users input
    any cpuid level and output the corresponding cpuid, in this way we can
    avoid to consider cpu differences and left this to userspace. We can
    also consider all the x86 platforms to do cpuid for every one.

    >
    > It's broken, because it is better for the userspace extractor to have
    > this logic than to stuff it into the kernel, where it sits hogging
    > unswappable memory at all times.

    It seems not to be very appropriate to let user space consider hardware
    details. /proc/cpuinfo should be an example to justify this.

    Is there any user application using /dev/cpu/*/cpuid? if no, i think it
    is feasible to provide an interface in the kernel.

    I noticed an application cpu-z on Windows, maybe we can clone it on
    Linux.
    >
    > -hpa


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    Yi Yang wrote:
    >>
    >> It's broken, because it doesn't take into account the fact that Intel
    >> broke CPUID level 4 and made it "repeating" (neither did the cpuid char
    >> device, because it predated the Intel braindamage; I've had a patch for
    >> it privately for a while, but didn't push it upstream because paravirt
    >> broke it royally and I wanted the situation to settle down.)
    >>
    >> It's broken, because the algorithm used to determine valid CPUID levels
    >> is incorrect; it fails to recognize any CPUID levels other than the main
    >> Intel and AMD ones, e.g. the Transmeta 0x8086xxxx (and sometimes more)
    >> and VIA 0xc000xxxx levels.

    > Thank you for pointing out these issues, i think we can let users input
    > any cpuid level and output the corresponding cpuid, in this way we can
    > avoid to consider cpu differences and left this to userspace. We can
    > also consider all the x86 platforms to do cpuid for every one.
    >
    >> It's broken, because it is better for the userspace extractor to have
    >> this logic than to stuff it into the kernel, where it sits hogging
    >> unswappable memory at all times.

    > It seems not to be very appropriate to let user space consider hardware
    > details. /proc/cpuinfo should be an example to justify this.


    /proc/cpuinfo represents what the kernel needs to know, so it reflects
    the kernel's interpretation of CPUID. There is no reason to interpret
    things in the kernel that the kernel doesn't need.

    > Is there any user application using /dev/cpu/*/cpuid? if no, i think it
    > is feasible to provide an interface in the kernel.


    Yes. It's called x86info, I believe.

    -hpa
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    On Tue, 2008-01-29 at 07:51 -0800, H. Peter Anvin wrote:
    > Yi Yang wrote:
    > > Current cpuid module will create a char device for every logical cpu,
    > > when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop,
    > > the root cause is that cpuid module doesn't decide wether a cpuid level
    > > is valid, it just uses an offset to denote cpuid level and take it to
    > > cpuid instruction, cpuid instruction will ignore it and return some data
    > >
    > > This patch uses sysfs to avoid limitless loop and provide more flexible
    > > interface for cpuid, please consider to merge to -mm tree in order to test.

    >
    > This is broken.
    >
    > Triple broken.
    >
    > It's broken, because it doesn't take into account the fact that Intel
    > broke CPUID level 4 and made it "repeating" (neither did the cpuid char
    > device, because it predated the Intel braindamage; I've had a patch for
    > it privately for a while, but didn't push it upstream because paravirt
    > broke it royally and I wanted the situation to settle down.)

    level 4 doesn't result in repeating on Intel CPU, cpuid module sets
    file offset to level, so cat /dev/cpu/*/cpuid will run cpuid instruction
    continuously.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    Yi Yang wrote:
    >>
    >> It's broken, because it doesn't take into account the fact that Intel
    >> broke CPUID level 4 and made it "repeating" (neither did the cpuid char
    >> device, because it predated the Intel braindamage; I've had a patch for
    >> it privately for a while, but didn't push it upstream because paravirt
    >> broke it royally and I wanted the situation to settle down.)


    > level 4 doesn't result in repeating on Intel CPU, cpuid module sets
    > file offset to level, so cat /dev/cpu/*/cpuid will run cpuid instruction
    > continuously.


    The issue is that Intel suddenly made CPUID ECX-sensitive, which there
    is no way to represent.

    As far as cat /dev/cpu/*/cpuid, that's a user error.

    -hpa

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    On Tue, 2008-01-29 at 23:17 -0800, H. Peter Anvin wrote:
    > Yi Yang wrote:
    > >>
    > >> It's broken, because it doesn't take into account the fact that Intel
    > >> broke CPUID level 4 and made it "repeating" (neither did the cpuid char
    > >> device, because it predated the Intel braindamage; I've had a patch for
    > >> it privately for a while, but didn't push it upstream because paravirt
    > >> broke it royally and I wanted the situation to settle down.)

    >
    > > level 4 doesn't result in repeating on Intel CPU, cpuid module sets
    > > file offset to level, so cat /dev/cpu/*/cpuid will run cpuid instruction
    > > continuously.

    >
    > The issue is that Intel suddenly made CPUID ECX-sensitive, which there
    > is no way to represent.

    Function cpuid has reset ecx to 0 immediate before calling to __cpuid,
    so this shouldn't be a problem now.

    in include/asm-x86/processor_32.h
    /*
    * Generic CPUID function
    * clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
    * resulting in stale register contents being returned.
    */
    static inline void cpuid(unsigned int op,
    unsigned int *eax, unsigned int *ebx,
    unsigned int *ecx, unsigned int *edx)
    {
    *eax = op;
    *ecx = 0;
    __cpuid(eax, ebx, ecx, edx);
    }
    >
    > As far as cat /dev/cpu/*/cpuid, that's a user error.
    >
    > -hpa
    >


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    Yi Yang wrote:
    > Function cpuid has reset ecx to 0 immediate before calling to __cpuid,
    > so this shouldn't be a problem now.


    Unless, of course, you want to get to the information for the higher
    CPUID levels.

    The easiest way to fix that would be to use cpuid_count() and let
    /dev/cpu/*/cpuid take the %ecx value in the high half of the offset.
    That would have minimal impact on the interface.

    -hpa
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    On Tue, 2008-01-29 at 09:44 +0100, Sam Ravnborg wrote:
    > > +
    > > +static struct notifier_block __cpuinitdata cpuid_sysfs_cpu_notifier = {
    > > + .notifier_call = cpuid_sysfs_cpu_callback,
    > > +};

    > Data is annotated _cpuintidata
    >
    > but
    >
    > > +

    > Data is annotated _cpuintidata
    >
    > > @@ -217,11 +445,14 @@ static void __exit cpuid_exit(void)
    > > {
    > > int cpu = 0;
    > >
    > > - for_each_online_cpu(cpu)
    > > + for_each_online_cpu(cpu) {
    > > cpuid_device_destroy(cpu);
    > > + remove_cpuid_sysfs(cpu);
    > > + }
    > > class_destroy(cpuid_class);
    > > unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
    > > unregister_hotcpu_notifier(&cpuid_class_cpu_notifier);
    > > + unregister_hotcpu_notifier(&cpuid_sysfs_cpu_notifier);

    >
    > used in an __exit function.
    >
    > You should have seen a Section mismatch warning for this.
    > The right fix is to annotate the cpuid_sysfs_cpu_notifier
    > with __initdata_refok (soon to be named __refdata)
    > Or even better to declare it const and use _refconst.

    I think __cpuinitdata is different from __initdata, i have tested it
    by insmod, rmmod, echo 0/1 > /sys/devices/system/cpu/cpu1/online
    repeatly, it hasn't any issue.

    >
    > Sam


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    On Wed, Jan 30, 2008 at 06:22:43AM +0800, Yi Yang wrote:
    > On Tue, 2008-01-29 at 09:44 +0100, Sam Ravnborg wrote:
    > > > +
    > > > +static struct notifier_block __cpuinitdata cpuid_sysfs_cpu_notifier = {
    > > > + .notifier_call = cpuid_sysfs_cpu_callback,
    > > > +};

    > > Data is annotated _cpuintidata
    > >
    > > but
    > >
    > > > +

    > > Data is annotated _cpuintidata
    > >
    > > > @@ -217,11 +445,14 @@ static void __exit cpuid_exit(void)
    > > > {
    > > > int cpu = 0;
    > > >
    > > > - for_each_online_cpu(cpu)
    > > > + for_each_online_cpu(cpu) {
    > > > cpuid_device_destroy(cpu);
    > > > + remove_cpuid_sysfs(cpu);
    > > > + }
    > > > class_destroy(cpuid_class);
    > > > unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
    > > > unregister_hotcpu_notifier(&cpuid_class_cpu_notifier);
    > > > + unregister_hotcpu_notifier(&cpuid_sysfs_cpu_notifier);

    > >
    > > used in an __exit function.
    > >
    > > You should have seen a Section mismatch warning for this.
    > > The right fix is to annotate the cpuid_sysfs_cpu_notifier
    > > with __initdata_refok (soon to be named __refdata)
    > > Or even better to declare it const and use _refconst.

    > I think __cpuinitdata is different from __initdata, i have tested it
    > by insmod, rmmod, echo 0/1 > /sys/devices/system/cpu/cpu1/online
    > repeatly, it hasn't any issue.


    __cpuinit & _cpuinitdata have over time been used for
    different purposes:
    a) To annotate code/data used in the init path and that in the
    non HOTPLUG_CPU case can be discarded after init.
    b) To annotate code/data used in the 'core' HOTPLUG_CPU
    functionality that isonly in use if HOTPLUG_CPU='y'


    The b) usage is questionable and the annotation
    of cpuid_sysfs_cpu_notifier beongs in the b) category.

    The correct solution would be to factor out the 'core'
    HOTPLUG_CPU=y code to a set of separate files and used to
    usual mechanishm in the Makefile to select when to include
    this code in the kernel.

    The improved section mismatch checks by modpost has just
    brought this issue to the attention and now you add code
    that does the wrong thing it is being discussed.

    Sam
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. [RFC PATCH 2.6.24] x86: add sysfs interface for cpuid module, try 2

    This patch is try 2, it should cover VIA/Cyrix and Transmeta Crusoe, please
    help to test it if anybody has VIA/Cyrix and Transmeta Crusoe. This patch just
    makes users get cpuid raw data more conveniently by
    /sys/devices/syste/cpu/cpu*/cpuid/* without using any userspace application.

    Current cpuid module will create a char device for every logical cpu,
    when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop,
    the root cause is that cpuid module doesn't decide wether a cpuid level
    is valid, it just uses an offset to denote cpuid level and take it to
    cpuid instruction, cpuid instruction will ignore it and return some data
    specific to cpu model, cpuid doesn't an error return value because it is
    void type. So cpuid module will execute cpuid continuously and return
    data although most of data make no sense.

    This patch tries to add a sysfs interface for cpuid, users can see all the
    available cpuid levels, specify a specific level and get cpuid corresponding
    to this cpuid level.

    For every logical cpu, this patch will create a cpuid directory under
    /sys/devices/system/cpu/cpu*/, there are three entries under cpuid:

    avail_levels cur_level cur_cpuid

    A user can get all the available cpuid levels from avail_levels, he/she can
    set one available cpuid level to cur_level, then he/she can get cpuid from
    cur_cpuid, cur_cpuid corresponds to cur_level.

    Here are some example output:
    [root@yangyi-dev /]# ls /sys/devices/system/cpu/cpu0/cpuid
    avail_levels cur_cpuid cur_level
    [root@yangyi-dev /]# cat /sys/devices/system/cpu/cpu0/cpuid/avail_levels
    0x00000000
    0x00000001
    0x00000002
    0x00000003
    0x00000004
    0x00000005
    0x00000006
    0x00000007
    0x00000008
    0x00000009
    0x0000000a
    0x80000000
    0x80000001
    0x80000002
    0x80000003
    0x80000004
    0x80000005
    0x80000006
    0x80000007
    0x80000008
    [root@yangyi-dev /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_level
    0x00000000
    [root@yangyi-dev /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
    0x0000000a 0x756e6547 0x6c65746e 0x49656e69
    [root@yangyi-dev /]# echo 0x80000007 > /sys/devices/system/cpu/cpu0/cpuid/cur_level
    [root@yangyi-dev /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
    0x00000000 0x00000000 0x00000000 0x00000000
    [root@yangyi-dev /]# echo 0x80000001 > /sys/devices/system/cpu/cpu0/cpuid/cur_level
    [root@yangyi-dev /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
    0x00000000 0x00000000 0x00000001 0x20100000
    [root@yangyi-dev /]# ls /sys/devices/system/cpu/cpu1/cpuid
    avail_levels cur_cpuid cur_level
    [root@yangyi-dev /]#

    Signed-off-by: Yi Yang
    ---
    cpuid.c | 255 ++++++++++++++++++++++++++++++++++++++++++++++++++ +++++++++++++-
    1 file changed, 254 insertions(+), 1 deletion(-)

    --- a/arch/x86/kernel/cpuid.c 2008-01-28 01:28:48.000000000 +0800
    +++ b/arch/x86/kernel/cpuid.c 2008-01-30 07:31:40.000000000 +0800
    @@ -175,6 +175,250 @@ static struct notifier_block __cpuinitda
    .notifier_call = cpuid_class_cpu_callback,
    };

    +struct cpuid_info {
    + int cpu;
    + unsigned int cur_level;
    + unsigned int min_level;
    + unsigned int max_level;
    + unsigned int min_ext_level;
    + unsigned int max_ext_level;
    + struct kobject kobj;
    +};
    +
    +struct cpuid_attr {
    + struct attribute attr;
    + ssize_t (*show)(struct cpuid_info *, char *);
    + ssize_t (*store)(struct cpuid_info *, const char *, size_t count);
    +};
    +
    +static struct cpuid_info cpuid_infos[NR_CPUS];
    +
    +static cpumask_t cpu_map = CPU_MASK_NONE;
    +
    +#define A32(a, b, c, d) (((d) << 24) + ((c) << 16) + ((b) << 8) + (a))
    +
    +static int is_centaur(u32 *data)
    +{
    + return data[1] == A32('C', 'e', 'n', 't') &&
    + data[3] == A32('a', 'u', 'r', 'H') &&
    + data[2] == A32('a', 'u', 'l', 's');
    +}
    +
    +static int is_transmeta(u32 *data)
    +{
    + return data[1] == A32('G', 'e', 'n', 'u') &&
    + data[3] == A32('i', 'n', 'e', 'T') &&
    + data[2] == A32('M', 'x', '8', '6');
    +}
    +
    +static ssize_t show_cur_level(struct cpuid_info *cpuid_info_p, char *buf)
    +{
    + return sprintf(buf, "0x%08x\n", cpuid_info_p->cur_level);
    +}
    +
    +static ssize_t store_cur_level(struct cpuid_info *cpuid_info_p,
    + const char *buf, size_t count)
    +{
    + char *p;
    + unsigned int val;
    + size_t len = 13;
    + char tmpbuf[16];
    +
    + if ((count > len) || (count <= 0))
    + return -EINVAL;
    +
    + len = count;
    + if (buf[count-1] == '\n')
    + len--;
    +
    + memcpy(tmpbuf, buf, len);
    + tmpbuf[len] = '\0';
    + val = simple_strtoul(tmpbuf, &p, 0);
    +
    + if (*p != '\0')
    + return -EINVAL;
    + if (((val >= cpuid_info_p->min_level)
    + && (val <= cpuid_info_p->max_level))
    + || ((val >= cpuid_info_p->min_ext_level)
    + && (val <= cpuid_info_p->max_ext_level))) {
    + cpuid_info_p->cur_level = val;
    + return count;
    + } else
    + return -EINVAL;
    +}
    +
    +static ssize_t show_avail_levels(struct cpuid_info *cpuid_info_p, char *buf)
    +{
    + unsigned int level;
    + ssize_t len = 0;
    +
    + level = cpuid_info_p->min_level;
    + while (level <= cpuid_info_p->max_level) {
    + len += sprintf(buf + len, "0x%08x\n", level);
    + level++;
    + }
    +
    + level = cpuid_info_p->min_ext_level;
    + while (level <= cpuid_info_p->max_ext_level) {
    + len += sprintf(buf + len, "0x%08x\n", level);
    + level++;
    + }
    + return len;
    +}
    +
    +static ssize_t show_cur_cpuid(struct cpuid_info *cpuid_info_p, char *buf)
    +{
    + u32 data[4];
    +
    + do_cpuid(cpuid_info_p->cpu, cpuid_info_p->cur_level, data);
    + return sprintf(buf, "0x%08x 0x%08x 0x%08x 0x%08x\n",
    + data[0], data[1], data[2], data[3]);
    +}
    +
    +static struct cpuid_attr cur_level =
    + __ATTR(cur_level, 0600, show_cur_level, store_cur_level);
    +static struct cpuid_attr avail_levels =
    + __ATTR(avail_levels, 0400, show_avail_levels, NULL);
    +static struct cpuid_attr cur_cpuid =
    + __ATTR(cur_cpuid, 0400, show_cur_cpuid, NULL);
    +
    +static struct attribute *default_attrs[] = {
    + &cur_level.attr,
    + &avail_levels.attr,
    + &cur_cpuid.attr,
    + NULL
    +};
    +#define to_cpuid_info(k) container_of(k, struct cpuid_info, kobj)
    +#define to_cpuid_attr(a) container_of(a, struct cpuid_attr, attr)
    +
    +static ssize_t check_cpu(struct cpuid_info *cpuid_info_p)
    +{
    + int cpu = cpuid_info_p->cpu;
    + struct cpuinfo_x86 *c = &cpu_data(cpu);
    +
    + if (cpu >= NR_CPUS || !cpu_online(cpu))
    + return -ENXIO; /* No such CPU */
    + if (c->cpuid_level < 0)
    + return -EIO; /* CPUID not supported */
    +
    + return 0;
    +}
    +
    +static ssize_t cpuid_show(struct kobject *kobj, struct attribute *attr,
    + char *buf)
    +{
    + ssize_t ret;
    + struct cpuid_attr *fattr = to_cpuid_attr(attr);
    + struct cpuid_info *cpuid_info_p = to_cpuid_info(kobj);
    +
    + ret = check_cpu(cpuid_info_p);
    + if (ret != 0)
    + return ret;
    +
    + return fattr->show(cpuid_info_p, buf);
    +}
    +
    +static ssize_t cpuid_store(struct kobject *kobj, struct attribute *attr,
    + const char *buf, size_t count)
    +{
    + struct cpuid_attr *fattr = to_cpuid_attr(attr);
    + struct cpuid_info *cpuid_info_p = to_cpuid_info(kobj);
    + ssize_t ret;
    +
    + ret = check_cpu(cpuid_info_p);
    + if (ret != 0)
    + return ret;
    +
    + ret = fattr->store ? fattr->store(cpuid_info_p, buf, count) : 0;
    + return ret;
    +}
    +
    +static void cpuid_sysfs_release(struct kobject *kobj)
    +{
    +}
    +
    +static struct sysfs_ops cpuid_sysfs_ops = {
    + .show = cpuid_show,
    + .store = cpuid_store,
    +};
    +
    +static struct kobj_type ktype_cpuid = {
    + .sysfs_ops = &cpuid_sysfs_ops,
    + .default_attrs = default_attrs,
    + .release = cpuid_sysfs_release,
    +};
    +
    +static __init_refok int create_cpuid_sysfs(int cpu)
    +{
    + struct sys_device *cpu_sys_dev = NULL;
    + u32 data[4];
    + int retval;
    + u32 ext_level = 0x80000000;
    +
    + cpu_sys_dev = get_cpu_sysdev(cpu);
    + if (cpu_sys_dev == NULL)
    + return -1;
    +
    + cpuid_infos[cpu].kobj.parent = &cpu_sys_dev->kobj;
    + kobject_set_name(&(cpuid_infos[cpu].kobj), "%s", "cpuid");
    + cpuid_infos[cpu].kobj.ktype = &ktype_cpuid;
    +
    + do_cpuid(cpu, 0, data);
    + cpuid_infos[cpu].cpu = cpu;
    + cpuid_infos[cpu].cur_level = 0;
    + cpuid_infos[cpu].min_level = 0;
    + cpuid_infos[cpu].max_level = data[0];
    +
    + if (is_centaur(data))
    + ext_level = 0xc0000000;
    + else if (is_transmeta(data))
    + ext_level = 0x80860000;
    +
    + do_cpuid(cpu, ext_level, data);
    + cpuid_infos[cpu].min_ext_level = ext_level;
    + cpuid_infos[cpu].max_ext_level = data[0];
    +
    + retval = kobject_register(&(cpuid_infos[cpu].kobj));
    + if (retval < 0) {
    + cpu_clear(cpu, cpu_map);
    + return retval;
    + }
    + cpu_set(cpu, cpu_map);
    + return 0;
    +}
    +
    +static void remove_cpuid_sysfs(int cpu)
    +{
    + if (cpu_isset(cpu, cpu_map)) {
    + cpu_clear(cpu, cpu_map);
    + kobject_unregister(&(cpuid_infos[cpu].kobj));
    + memset(&cpuid_infos[cpu], 0, sizeof(struct cpuid_info));
    + }
    +}
    +
    +static int __init_refok cpuid_sysfs_cpu_callback(struct notifier_block *nfb,
    + unsigned long action,
    + void *hcpu)
    +{
    + unsigned int cpu = (unsigned long)hcpu;
    +
    + switch (action) {
    + case CPU_ONLINE:
    + case CPU_ONLINE_FROZEN:
    + create_cpuid_sysfs(cpu);
    + break;
    + case CPU_DEAD:
    + case CPU_DEAD_FROZEN:
    + remove_cpuid_sysfs(cpu);
    + break;
    + }
    + return NOTIFY_OK;
    +}
    +
    +static struct notifier_block __initdata_refok cpuid_sysfs_cpu_notifier = {
    + .notifier_call = cpuid_sysfs_cpu_callback,
    +};
    +
    static int __init cpuid_init(void)
    {
    int i, err = 0;
    @@ -195,8 +439,13 @@ static int __init cpuid_init(void)
    err = cpuid_device_create(i);
    if (err != 0)
    goto out_class;
    +
    + err = create_cpuid_sysfs(i);
    + if (err != 0)
    + goto out_class;
    }
    register_hotcpu_notifier(&cpuid_class_cpu_notifier);
    + register_hotcpu_notifier(&cpuid_sysfs_cpu_notifier);

    err = 0;
    goto out;
    @@ -205,6 +454,7 @@ out_class:
    i = 0;
    for_each_online_cpu(i) {
    cpuid_device_destroy(i);
    + remove_cpuid_sysfs(i);
    }
    class_destroy(cpuid_class);
    out_chrdev:
    @@ -217,11 +467,14 @@ static void __exit cpuid_exit(void)
    {
    int cpu = 0;

    - for_each_online_cpu(cpu)
    + for_each_online_cpu(cpu) {
    cpuid_device_destroy(cpu);
    + remove_cpuid_sysfs(cpu);
    + }
    class_destroy(cpuid_class);
    unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
    unregister_hotcpu_notifier(&cpuid_class_cpu_notifier);
    + unregister_hotcpu_notifier(&cpuid_sysfs_cpu_notifier);
    }

    module_init(cpuid_init);


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [RFC PATCH 2.6.24] x86: add sysfs interface for cpuid module, try 2

    Yi Yang wrote:
    > This patch is try 2, it should cover VIA/Cyrix and Transmeta Crusoe, please
    > help to test it if anybody has VIA/Cyrix and Transmeta Crusoe. This patch just
    > makes users get cpuid raw data more conveniently by
    > /sys/devices/syste/cpu/cpu*/cpuid/* without using any userspace application.


    Still completely useless garbage.

    -hpa
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. [PATCH 2.6.24] Add new string functions real_strtoul and change kernel params to use them

    Currently, for every sysfs node, the callers will be responsible for
    implementing store operation, so many many callers are doing duplicate
    things to validate input, they have the same mistakes because they are
    calling simple_strtol/ul/ll/ull, especially for module params, they are
    just numeric, but you can echo such values as 0x1234xxx, 07777888 and
    1234aaa, for these cases, module params store operation just ignores
    successive invalid char and converts prefix part to a numeric although
    input is actually invalid.

    This patch tries to fix the aforementioned issues and implements real_strtox
    serial functions, kernel/params.c uses them to strictly validate input,
    so module params will reject such values as 0x1234xxxx and returns an error:

    write error: Invalid argument

    Any modules which export numeric sysfs node can use real_strtox instead of
    simple_strtox to reject any invalid input. Please consider to merge to -mm tree
    in order to test.

    Here are some test results:

    Before applying this patch:

    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]#


    After applying this patch:

    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]#


    Signed-off-by: Yi Yang
    ---
    include/linux/kernel.h | 4 ++++
    kernel/params.c | 20 ++++++++++----------
    lib/vsprintf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
    3 files changed, 60 insertions(+), 10 deletions(-)

    --- a/include/linux/kernel.h 2008-01-31 00:41:46.000000000 +0800
    +++ b/include/linux/kernel.h 2008-01-31 01:12:33.000000000 +0800
    @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
    extern long simple_strtol(const char *,char **,unsigned int);
    extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
    extern long long simple_strtoll(const char *,char **,unsigned int);
    +extern int real_strtoul(const char *, unsigned int, unsigned long *);
    +extern int real_strtol(const char *, unsigned int, long *);
    +extern int real_strtoull(const char *, unsigned int, unsigned long long *);
    +extern int real_strtoll(const char *, unsigned int, long long *);
    extern int sprintf(char * buf, const char * fmt, ...)
    __attribute__ ((format (printf, 2, 3)));
    extern int vsprintf(char *buf, const char *, va_list)
    --- a/lib/vsprintf.c 2008-01-30 08:13:03.000000000 +0800
    +++ b/lib/vsprintf.c 2008-01-31 05:19:31.000000000 +0800
    @@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
    return simple_strtoull(cp,endp,base);
    }

    +#define define_real_strtoux(type, valtype) \
    +int real_strtou##type(const char *cp, unsigned int base, valtype *res) \
    +{ \
    + char *tail; \
    + valtype val; \
    + size_t len; \
    + \
    + *res = 0; \
    + len = strlen(cp); \
    + if (len == 0) \
    + return -EINVAL; \
    + \
    + val = simple_strtoul(cp, &tail, base); \
    + if ((*tail == '\0') || \
    + (len == (size_t)(tail - cp) + 1) && (*tail == '\n')) { \
    + *res = val; \
    + return 0; \
    + } \
    + \
    + return -EINVAL; \
    +} \
    +
    +#define define_real_strtox(type, valtype) \
    +int real_strto##type(const char *cp, unsigned int base, valtype *res) \
    +{ \
    + int ret; \
    + if (*cp == '-') { \
    + ret = real_strtou##type(cp+1, base, res); \
    + if (ret != 0) \
    + *res = -(*res); \
    + } else \
    + ret = real_strtou##type(cp+1, base, res); \
    + \
    + return ret; \
    +} \
    +
    +define_real_strtoux(l, unsigned long)
    +define_real_strtox(l, long)
    +define_real_strtoux(ll, unsigned long long)
    +define_real_strtox(ll, long long)
    +
    +EXPORT_SYMBOL(real_strtoul);
    +EXPORT_SYMBOL(real_strtol);
    +EXPORT_SYMBOL(real_strtoll);
    +EXPORT_SYMBOL(real_strtoull);
    +
    static int skip_atoi(const char **s)
    {
    int i=0;
    --- a/kernel/params.c 2008-01-31 00:44:44.000000000 +0800
    +++ b/kernel/params.c 2008-01-31 01:11:51.000000000 +0800
    @@ -180,12 +180,12 @@ int parse_args(const char *name,
    #define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn) \
    int param_set_##name(const char *val, struct kernel_param *kp) \
    { \
    - char *endp; \
    tmptype l; \
    + int ret; \
    \
    if (!val) return -EINVAL; \
    - l = strtolfn(val, &endp, 0); \
    - if (endp == val || ((type)l != l)) \
    + ret = strtolfn(val, 0, &l); \
    + if (ret == -EINVAL || ((type)l != l)) \
    return -EINVAL; \
    *((type *)kp->arg) = l; \
    return 0; \
    @@ -195,13 +195,13 @@ int parse_args(const char *name,
    return sprintf(buffer, format, *((type *)kp->arg)); \
    }

    -STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(short, short, "%hi", long, simple_strtol);
    -STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(int, int, "%i", long, simple_strtol);
    -STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(long, long, "%li", long, simple_strtol);
    -STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, simple_strtoul);
    +STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, real_strtoul);
    +STANDARD_PARAM_DEF(short, short, "%hi", long, real_strtol);
    +STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, real_strtoul);
    +STANDARD_PARAM_DEF(int, int, "%i", long, real_strtol);
    +STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, real_strtoul);
    +STANDARD_PARAM_DEF(long, long, "%li", long, real_strtol);
    +STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, real_strtoul);

    int param_set_charp(const char *val, struct kernel_param *kp)
    {


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [PATCH 2.6.24] Add new string functions real_strtoul and change kernel params to use them

    On Thu, 31 Jan 2008 09:18:22 +0800 Yi Yang wrote:

    > Currently, for every sysfs node, the callers will be responsible for
    > implementing store operation, so many many callers are doing duplicate
    > things to validate input, they have the same mistakes because they are
    > calling simple_strtol/ul/ll/ull, especially for module params, they are
    > just numeric, but you can echo such values as 0x1234xxx, 07777888 and
    > 1234aaa, for these cases, module params store operation just ignores
    > successive invalid char and converts prefix part to a numeric although
    > input is actually invalid.
    >
    > This patch tries to fix the aforementioned issues and implements real_strtox
    > serial functions, kernel/params.c uses them to strictly validate input,
    > so module params will reject such values as 0x1234xxxx and returns an error:


    How about a prefix of safe_ or strict_ or something other than real_?
    real_ sounds too much like a real number function string parser.

    > write error: Invalid argument
    >
    > Any modules which export numeric sysfs node can use real_strtox instead of
    > simple_strtox to reject any invalid input. Please consider to merge to -mm tree
    > in order to test.
    >
    > Here are some test results:
    >
    > Before applying this patch:
    >
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]#
    >
    >
    > After applying this patch:
    >
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]#
    >
    >
    > Signed-off-by: Yi Yang
    > ---
    > include/linux/kernel.h | 4 ++++
    > kernel/params.c | 20 ++++++++++----------
    > lib/vsprintf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
    > 3 files changed, 60 insertions(+), 10 deletions(-)
    >
    > --- a/include/linux/kernel.h 2008-01-31 00:41:46.000000000 +0800
    > +++ b/include/linux/kernel.h 2008-01-31 01:12:33.000000000 +0800
    > @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
    > extern long simple_strtol(const char *,char **,unsigned int);
    > extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
    > extern long long simple_strtoll(const char *,char **,unsigned int);
    > +extern int real_strtoul(const char *, unsigned int, unsigned long *);
    > +extern int real_strtol(const char *, unsigned int, long *);
    > +extern int real_strtoull(const char *, unsigned int, unsigned long long *);
    > +extern int real_strtoll(const char *, unsigned int, long long *);
    > extern int sprintf(char * buf, const char * fmt, ...)
    > __attribute__ ((format (printf, 2, 3)));
    > extern int vsprintf(char *buf, const char *, va_list)
    > --- a/lib/vsprintf.c 2008-01-30 08:13:03.000000000 +0800
    > +++ b/lib/vsprintf.c 2008-01-31 05:19:31.000000000 +0800
    > @@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
    > return simple_strtoull(cp,endp,base);
    > }
    >
    > +#define define_real_strtoux(type, valtype) \
    > +int real_strtou##type(const char *cp, unsigned int base, valtype *res) \
    > +{ \
    > + char *tail; \
    > + valtype val; \
    > + size_t len; \
    > + \
    > + *res = 0; \
    > + len = strlen(cp); \
    > + if (len == 0) \
    > + return -EINVAL; \
    > + \
    > + val = simple_strtoul(cp, &tail, base); \
    > + if ((*tail == '\0') || \
    > + (len == (size_t)(tail - cp) + 1) && (*tail == '\n')) { \
    > + *res = val; \
    > + return 0; \
    > + } \
    > + \
    > + return -EINVAL; \
    > +} \
    > +
    > +#define define_real_strtox(type, valtype) \
    > +int real_strto##type(const char *cp, unsigned int base, valtype *res) \
    > +{ \
    > + int ret; \
    > + if (*cp == '-') { \
    > + ret = real_strtou##type(cp+1, base, res); \
    > + if (ret != 0) \
    > + *res = -(*res); \
    > + } else \
    > + ret = real_strtou##type(cp+1, base, res); \
    > + \
    > + return ret; \
    > +} \
    > +
    > +define_real_strtoux(l, unsigned long)
    > +define_real_strtox(l, long)
    > +define_real_strtoux(ll, unsigned long long)
    > +define_real_strtox(ll, long long)
    > +
    > +EXPORT_SYMBOL(real_strtoul);
    > +EXPORT_SYMBOL(real_strtol);
    > +EXPORT_SYMBOL(real_strtoll);
    > +EXPORT_SYMBOL(real_strtoull);
    > +
    > static int skip_atoi(const char **s)
    > {
    > int i=0;
    > --- a/kernel/params.c 2008-01-31 00:44:44.000000000 +0800
    > +++ b/kernel/params.c 2008-01-31 01:11:51.000000000 +0800
    > @@ -180,12 +180,12 @@ int parse_args(const char *name,
    > #define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn) \
    > int param_set_##name(const char *val, struct kernel_param *kp) \
    > { \
    > - char *endp; \
    > tmptype l; \
    > + int ret; \
    > \
    > if (!val) return -EINVAL; \
    > - l = strtolfn(val, &endp, 0); \
    > - if (endp == val || ((type)l != l)) \
    > + ret = strtolfn(val, 0, &l); \
    > + if (ret == -EINVAL || ((type)l != l)) \
    > return -EINVAL; \
    > *((type *)kp->arg) = l; \
    > return 0; \
    > @@ -195,13 +195,13 @@ int parse_args(const char *name,
    > return sprintf(buffer, format, *((type *)kp->arg)); \
    > }
    >
    > -STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, simple_strtoul);
    > -STANDARD_PARAM_DEF(short, short, "%hi", long, simple_strtol);
    > -STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, simple_strtoul);
    > -STANDARD_PARAM_DEF(int, int, "%i", long, simple_strtol);
    > -STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, simple_strtoul);
    > -STANDARD_PARAM_DEF(long, long, "%li", long, simple_strtol);
    > -STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, simple_strtoul);
    > +STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, real_strtoul);
    > +STANDARD_PARAM_DEF(short, short, "%hi", long, real_strtol);
    > +STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, real_strtoul);
    > +STANDARD_PARAM_DEF(int, int, "%i", long, real_strtol);
    > +STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, real_strtoul);
    > +STANDARD_PARAM_DEF(long, long, "%li", long, real_strtol);
    > +STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, real_strtoul);
    >
    > int param_set_charp(const char *val, struct kernel_param *kp)
    > {
    >
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/
    >



    ---
    ~Randy
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH 2.6.24] Add new string functions real_strtoul and change kernel params to use them

    On Thu, 2008-01-31 at 09:03 -0800, Randy Dunlap wrote:
    > On Thu, 31 Jan 2008 09:18:22 +0800 Yi Yang wrote:
    >
    > > Currently, for every sysfs node, the callers will be responsible for
    > > implementing store operation, so many many callers are doing duplicate
    > > things to validate input, they have the same mistakes because they are
    > > calling simple_strtol/ul/ll/ull, especially for module params, they are
    > > just numeric, but you can echo such values as 0x1234xxx, 07777888 and
    > > 1234aaa, for these cases, module params store operation just ignores
    > > successive invalid char and converts prefix part to a numeric although
    > > input is actually invalid.
    > >
    > > This patch tries to fix the aforementioned issues and implements real_strtox
    > > serial functions, kernel/params.c uses them to strictly validate input,
    > > so module params will reject such values as 0x1234xxxx and returns an error:

    >
    > How about a prefix of safe_ or strict_ or something other than real_?
    > real_ sounds too much like a real number function string parser.
    >

    I named it as strict_ at the beginning, but it results in some alignment
    issues checkpatch.pl will always warn, i don't know if warnings will
    make the patch out of the door.

    In kernel/params.c, STANDARD_PARAM_DEF(a function definition macro) will
    be over 80 chars, is it correct coding style to split it to two lines?

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [PATCH 2.6.24] Add new string functions real_strtoul and change kernel params to use them

    On Fri, 01 Feb 2008 00:30:17 +0800 Yi Yang wrote:

    > On Thu, 2008-01-31 at 09:03 -0800, Randy Dunlap wrote:
    > > On Thu, 31 Jan 2008 09:18:22 +0800 Yi Yang wrote:
    > >
    > > > Currently, for every sysfs node, the callers will be responsible for
    > > > implementing store operation, so many many callers are doing duplicate
    > > > things to validate input, they have the same mistakes because they are
    > > > calling simple_strtol/ul/ll/ull, especially for module params, they are
    > > > just numeric, but you can echo such values as 0x1234xxx, 07777888 and
    > > > 1234aaa, for these cases, module params store operation just ignores
    > > > successive invalid char and converts prefix part to a numeric although
    > > > input is actually invalid.
    > > >
    > > > This patch tries to fix the aforementioned issues and implements real_strtox
    > > > serial functions, kernel/params.c uses them to strictly validate input,
    > > > so module params will reject such values as 0x1234xxxx and returns an error:

    > >
    > > How about a prefix of safe_ or strict_ or something other than real_?
    > > real_ sounds too much like a real number function string parser.
    > >

    > I named it as strict_ at the beginning, but it results in some alignment
    > issues checkpatch.pl will always warn, i don't know if warnings will
    > make the patch out of the door.
    >
    > In kernel/params.c, STANDARD_PARAM_DEF(a function definition macro) will
    > be over 80 chars, is it correct coding style to split it to two lines?


    Yes, if it can be done cleanly. Otherwise just leave it long (IMHO).

    ---
    ~Randy
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. [PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 2

    This patch is a resend, it changes previous name "real_" to "strict_"
    according to Randy Dunlap's feedback. Please consider to apply. thanks.

    Currently, for every sysfs node, the callers will be responsible for
    implementing store operation, so many many callers are doing duplicate
    things to validate input, they have the same mistakes because they are
    calling simple_strtol/ul/ll/uul, especially for module params, they are
    just numeric, but you can echo such values as 0x1234xxx, 07777888 and
    1234aaa, for these cases, module params store operation just ignores
    succesive invalid char and converts prefix part to a numeric although
    input is acctually invalid.

    This patch tries to fix the aforementioned issues and implements strict_strtox
    serial functions, kernel/params.c uses them to strictly validate input,
    so module params will reject such values as 0x1234xxxx and returns an error:

    write error: Invalid argument

    Any modules which export numeric sysfs node can use strict_strtox instead of
    simple_strtox to reject any invalid input.

    Here are some test results:

    Before applying this patch:

    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]#


    After applying this patch:

    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]#


    Signed-off-by: Yi Yang
    ---
    include/linux/kernel.h | 4 ++++
    kernel/params.c | 20 ++++++++++----------
    lib/vsprintf.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
    3 files changed, 60 insertions(+), 10 deletions(-)

    --- a/include/linux/kernel.h 2008-01-31 00:41:46.000000000 +0800
    --- b/include/linux/kernel.h 2008-02-01 04:30:49.000000000 +0800
    @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
    extern long simple_strtol(const char *,char **,unsigned int);
    extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
    extern long long simple_strtoll(const char *,char **,unsigned int);
    +extern int strict_strtoul(const char *, unsigned int, unsigned long *);
    +extern int strict_strtol(const char *, unsigned int, long *);
    +extern int strict_strtoull(const char *, unsigned int, unsigned long long *);
    +extern int strict_strtoll(const char *, unsigned int, long long *);
    extern int sprintf(char * buf, const char * fmt, ...)
    __attribute__ ((format (printf, 2, 3)));
    extern int vsprintf(char *buf, const char *, va_list)
    --- a/lib/vsprintf.c 2008-01-30 08:13:03.000000000 +0800
    --- b/lib/vsprintf.c 2008-02-01 04:33:27.000000000 +0800
    @@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
    return simple_strtoull(cp,endp,base);
    }

    +#define define_strict_strtoux(type, valtype) \
    +int strict_strtou##type(const char *cp, unsigned int base, valtype *res)\
    +{ \
    + char *tail; \
    + valtype val; \
    + size_t len; \
    + \
    + *res = 0; \
    + len = strlen(cp); \
    + if (len == 0) \
    + return -EINVAL; \
    + \
    + val = simple_strtoul(cp, &tail, base); \
    + if ((*tail == '\0') || \
    + (len == (size_t)(tail - cp) + 1) && (*tail == '\n')) { \
    + *res = val; \
    + return 0; \
    + } \
    + \
    + return -EINVAL; \
    +} \
    +
    +#define define_strict_strtox(type, valtype) \
    +int strict_strto##type(const char *cp, unsigned int base, valtype *res) \
    +{ \
    + int ret; \
    + if (*cp == '-') { \
    + ret = strict_strtou##type(cp+1, base, res); \
    + if (ret != 0) \
    + *res = -(*res); \
    + } else \
    + ret = strict_strtou##type(cp+1, base, res); \
    + \
    + return ret; \
    +} \
    +
    +define_strict_strtoux(l, unsigned long)
    +define_strict_strtox(l, long)
    +define_strict_strtoux(ll, unsigned long long)
    +define_strict_strtox(ll, long long)
    +
    +EXPORT_SYMBOL(strict_strtoul);
    +EXPORT_SYMBOL(strict_strtol);
    +EXPORT_SYMBOL(strict_strtoll);
    +EXPORT_SYMBOL(strict_strtoull);
    +
    static int skip_atoi(const char **s)
    {
    int i=0;
    --- a/kernel/params.c 2008-01-31 00:44:44.000000000 +0800
    --- b/kernel/params.c 2008-02-01 04:32:12.000000000 +0800
    @@ -180,12 +180,12 @@ int parse_args(const char *name,
    #define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn) \
    int param_set_##name(const char *val, struct kernel_param *kp) \
    { \
    - char *endp; \
    tmptype l; \
    + int ret; \
    \
    if (!val) return -EINVAL; \
    - l = strtolfn(val, &endp, 0); \
    - if (endp == val || ((type)l != l)) \
    + ret = strtolfn(val, 0, &l); \
    + if (ret == -EINVAL || ((type)l != l)) \
    return -EINVAL; \
    *((type *)kp->arg) = l; \
    return 0; \
    @@ -195,13 +195,13 @@ int parse_args(const char *name,
    return sprintf(buffer, format, *((type *)kp->arg)); \
    }

    -STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(short, short, "%hi", long, simple_strtol);
    -STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(int, int, "%i", long, simple_strtol);
    -STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(long, long, "%li", long, simple_strtol);
    -STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, simple_strtoul);
    +STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, strict_strtoul);
    +STANDARD_PARAM_DEF(short, short, "%hi", long, strict_strtol);
    +STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, strict_strtoul);
    +STANDARD_PARAM_DEF(int, int, "%i", long, strict_strtol);
    +STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, strict_strtoul);
    +STANDARD_PARAM_DEF(long, long, "%li", long, strict_strtol);
    +STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);

    int param_set_charp(const char *val, struct kernel_param *kp)
    {


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 2

    On Fri, 01 Feb 2008 06:11:43 +0800 Yi Yang wrote:

    > This patch is a resend, it changes previous name "real_" to "strict_"
    > according to Randy Dunlap's feedback. Please consider to apply. thanks.
    >
    > Currently, for every sysfs node, the callers will be responsible for
    > implementing store operation, so many many callers are doing duplicate
    > things to validate input, they have the same mistakes because they are
    > calling simple_strtol/ul/ll/uul, especially for module params, they are
    > just numeric, but you can echo such values as 0x1234xxx, 07777888 and
    > 1234aaa, for these cases, module params store operation just ignores
    > succesive invalid char and converts prefix part to a numeric although
    > input is acctually invalid.
    >
    > This patch tries to fix the aforementioned issues and implements strict_strtox
    > serial functions, kernel/params.c uses them to strictly validate input,
    > so module params will reject such values as 0x1234xxxx and returns an error:
    >
    > write error: Invalid argument
    >
    > Any modules which export numeric sysfs node can use strict_strtox instead of
    > simple_strtox to reject any invalid input.
    >
    > Here are some test results:
    >
    > Before applying this patch:
    >
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]#
    >
    >
    > After applying this patch:
    >
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    > -bash: echo: write error: Invalid argument
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
    > [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    > 4096
    > [root@yangyi-dev /]#
    >


    Oh dear. So if someone has an existing script which does

    echo 0x1000g > /sys/module/e1000/parameters/copybreak

    we just broke their setup.

    This is what happens when we screw up kernel interfaces: we should remain
    bug-for-bug compatible with earlier releases. argh, we suck

    I'm inclined to merge it anyway - everyone already knows we suck, and
    not many people will be affected and they suck too.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 2

    On Fri, 01 Feb 2008 06:11:43 +0800 Yi Yang wrote:

    > --- a/include/linux/kernel.h 2008-01-31 00:41:46.000000000 +0800
    > --- b/include/linux/kernel.h 2008-02-01 04:30:49.000000000 +0800


    This isn't a patch. I wonder how that happened?
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 2

    On Fri, 01 Feb 2008 06:11:43 +0800 Yi Yang wrote:

    > --- a/include/linux/kernel.h 2008-01-31 00:41:46.000000000 +0800
    > --- b/include/linux/kernel.h 2008-02-01 04:30:49.000000000 +0800
    > @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
    > extern long simple_strtol(const char *,char **,unsigned int);
    > extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
    > extern long long simple_strtoll(const char *,char **,unsigned int);
    > +extern int strict_strtoul(const char *, unsigned int, unsigned long *);
    > +extern int strict_strtol(const char *, unsigned int, long *);
    > +extern int strict_strtoull(const char *, unsigned int, unsigned long long *);
    > +extern int strict_strtoll(const char *, unsigned int, long long *);
    > extern int sprintf(char * buf, const char * fmt, ...)
    > __attribute__ ((format (printf, 2, 3)));
    > extern int vsprintf(char *buf, const char *, va_list)
    > --- a/lib/vsprintf.c 2008-01-30 08:13:03.000000000 +0800
    > --- b/lib/vsprintf.c 2008-02-01 04:33:27.000000000 +0800
    > @@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
    > return simple_strtoull(cp,endp,base);
    > }
    >
    > +#define define_strict_strtoux(type, valtype) \
    > +int strict_strtou##type(const char *cp, unsigned int base, valtype *res)\
    > +{ \
    > + char *tail; \
    > + valtype val; \
    > + size_t len; \
    > + \
    > + *res = 0; \
    > + len = strlen(cp); \
    > + if (len == 0) \
    > + return -EINVAL; \
    > + \
    > + val = simple_strtoul(cp, &tail, base); \
    > + if ((*tail == '\0') || \
    > + (len == (size_t)(tail - cp) + 1) && (*tail == '\n')) { \
    > + *res = val; \
    > + return 0; \
    > + } \
    > + \
    > + return -EINVAL; \
    > +} \
    > +
    > +#define define_strict_strtox(type, valtype) \
    > +int strict_strto##type(const char *cp, unsigned int base, valtype *res) \
    > +{ \
    > + int ret; \
    > + if (*cp == '-') { \
    > + ret = strict_strtou##type(cp+1, base, res); \
    > + if (ret != 0) \
    > + *res = -(*res); \
    > + } else \
    > + ret = strict_strtou##type(cp+1, base, res); \
    > + \
    > + return ret; \
    > +} \
    > +
    > +define_strict_strtoux(l, unsigned long)
    > +define_strict_strtox(l, long)
    > +define_strict_strtoux(ll, unsigned long long)
    > +define_strict_strtox(ll, long long)
    > +
    > +EXPORT_SYMBOL(strict_strtoul);
    > +EXPORT_SYMBOL(strict_strtol);
    > +EXPORT_SYMBOL(strict_strtoll);
    > +EXPORT_SYMBOL(strict_strtoull);


    These are global, exported-to-modules library functions. Documentation is
    compulsory (as far as I'm concerned).

    Could you please add a comment block in there which at least explains how
    they differ from the other strto* functions?


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. [PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 3

    Andrew, i'm really very very sorry for those mistakes, here is the latest, it
    adds documentation for every new function, please use it to replace the patch
    in -mm tree you added just now:

    add-new-string-functions-strict_strto-and-convert-kernel-params-to-use-them.patch

    Thank you, Andrew and Randy.

    Currently, for every sysfs node, the callers will be responsible for
    implementing store operation, so many many callers are doing duplicate
    things to validate input, they have the same mistakes because they are
    calling simple_strtol/ul/ll/uul, especially for module params, they are
    just numeric, but you can echo such values as 0x1234xxx, 07777888 and
    1234aaa, for these cases, module params store operation just ignores
    succesive invalid char and converts prefix part to a numeric although
    input is acctually invalid.

    This patch tries to fix the aforementioned issues and implements strict_strtox
    serial functions, kernel/params.c uses them to strictly validate input,
    so module params will reject such values as 0x1234xxxx and returns an error:

    write error: Invalid argument

    Any modules which export numeric sysfs node can use strict_strtox instead of
    simple_strtox to reject any invalid input. Please consider to merge to -mm tree
    in order to test.

    Here are some test results:

    Before applying this patch:

    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]#


    After applying this patch:

    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak
    -bash: echo: write error: Invalid argument
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
    [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak
    4096
    [root@yangyi-dev /]#


    Signed-off-by: Yi Yang
    ---
    include/linux/kernel.h | 4 +
    kernel/params.c | 20 +++----
    lib/vsprintf.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++
    3 files changed, 137 insertions(+), 10 deletions(-)

    --- a/include/linux/kernel.h 2008-01-31 00:41:46.000000000 +0800
    +++ b/include/linux/kernel.h 2008-02-01 04:30:49.000000000 +0800
    @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
    extern long simple_strtol(const char *,char **,unsigned int);
    extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
    extern long long simple_strtoll(const char *,char **,unsigned int);
    +extern int strict_strtoul(const char *, unsigned int, unsigned long *);
    +extern int strict_strtol(const char *, unsigned int, long *);
    +extern int strict_strtoull(const char *, unsigned int, unsigned long long *);
    +extern int strict_strtoll(const char *, unsigned int, long long *);
    extern int sprintf(char * buf, const char * fmt, ...)
    __attribute__ ((format (printf, 2, 3)));
    extern int vsprintf(char *buf, const char *, va_list)
    --- a/lib/vsprintf.c 2008-01-30 08:13:03.000000000 +0800
    +++ b/lib/vsprintf.c 2008-02-01 07:36:57.000000000 +0800
    @@ -126,6 +126,129 @@ long long simple_strtoll(const char *cp,
    return simple_strtoull(cp,endp,base);
    }

    +
    +/**
    + * strict_strtoul - convert a string to an unsigned long strictly
    + * @cp: The string to be converted
    + * @base: The number base to use
    + * @res: The converted result value
    + *
    + * strict_strtoul converts a string to an unsigned long only if the
    + * string is really an unsigned long string, any string containing
    + * any invalid char at the tail will be rejected and -EINVAL is returned,
    + * only a newline char at the tail is acceptible because people generally
    + * change a module parameter in the following way:
    + *
    + * echo 1024 > /sys/module/e1000/parameters/copybreak
    + *
    + * echo will append a newline to the tail.
    + *
    + * It returns 0 if conversion is successful and *res is set to the converted
    + * value, otherwise it returns -EINVAL and *res is set to 0.
    + *
    + * simple_strtoul just ignores the successive invalid characters and
    + * return the converted value of prefix part of the string.
    + */
    +int strict_strtoul(const char *cp, unsigned int base, unsigned long *res);
    +
    +/**
    + * strict_strtol - convert a string to a long strictly
    + * @cp: The string to be converted
    + * @base: The number base to use
    + * @res: The converted result value
    + *
    + * strict_strtol is similiar to strict_strtoul, but it allows the first
    + * character of a string is '-'.
    + *
    + * It returns 0 if conversion is successful and *res is set to the converted
    + * value, otherwise it returns -EINVAL and *res is set to 0.
    + */
    +int strict_strtol(const char *cp, unsigned int base, long *res);
    +
    +/**
    + * strict_strtoull - convert a string to an unsigned long long strictly
    + * @cp: The string to be converted
    + * @base: The number base to use
    + * @res: The converted result value
    + *
    + * strict_strtoull converts a string to an unsigned long long only if the
    + * string is really an unsigned long long string, any string containing
    + * any invalid char at the tail will be rejected and -EINVAL is returned,
    + * only a newline char at the tail is acceptible because people generally
    + * change a module parameter in the following way:
    + *
    + * echo 1024 > /sys/module/e1000/parameters/copybreak
    + *
    + * echo will append a newline to the tail of the string.
    + *
    + * It returns 0 if conversion is successful and *res is set to the converted
    + * value, otherwise it returns -EINVAL and *res is set to 0.
    + *
    + * simple_strtoull just ignores the successive invalid characters and
    + * return the converted value of prefix part of the string.
    + */
    +int strict_strtoull(const char *cp, unsigned int base, unsigned long long *res);
    +
    +/**
    + * strict_strtoll - convert a string to a long long strictly
    + * @cp: The string to be converted
    + * @base: The number base to use
    + * @res: The converted result value
    + *
    + * strict_strtoll is similiar to strict_strtoull, but it allows the first
    + * character of a string is '-'.
    + *
    + * It returns 0 if conversion is successful and *res is set to the converted
    + * value, otherwise it returns -EINVAL and *res is set to 0.
    + */
    +int strict_strtoll(const char *cp, unsigned int base, long long *res);
    +
    +#define define_strict_strtoux(type, valtype) \
    +int strict_strtou##type(const char *cp, unsigned int base, valtype *res)\
    +{ \
    + char *tail; \
    + valtype val; \
    + size_t len; \
    + \
    + *res = 0; \
    + len = strlen(cp); \
    + if (len == 0) \
    + return -EINVAL; \
    + \
    + val = simple_strtoul(cp, &tail, base); \
    + if ((*tail == '\0') || \
    + (len == (size_t)(tail - cp) + 1) && (*tail == '\n')) { \
    + *res = val; \
    + return 0; \
    + } \
    + \
    + return -EINVAL; \
    +} \
    +
    +#define define_strict_strtox(type, valtype) \
    +int strict_strto##type(const char *cp, unsigned int base, valtype *res) \
    +{ \
    + int ret; \
    + if (*cp == '-') { \
    + ret = strict_strtou##type(cp+1, base, res); \
    + if (ret != 0) \
    + *res = -(*res); \
    + } else \
    + ret = strict_strtou##type(cp+1, base, res); \
    + \
    + return ret; \
    +} \
    +
    +define_strict_strtoux(l, unsigned long)
    +define_strict_strtox(l, long)
    +define_strict_strtoux(ll, unsigned long long)
    +define_strict_strtox(ll, long long)
    +
    +EXPORT_SYMBOL(strict_strtoul);
    +EXPORT_SYMBOL(strict_strtol);
    +EXPORT_SYMBOL(strict_strtoll);
    +EXPORT_SYMBOL(strict_strtoull);
    +
    static int skip_atoi(const char **s)
    {
    int i=0;
    --- a/kernel/params.c 2008-01-31 00:44:44.000000000 +0800
    +++ b/kernel/params.c 2008-02-01 04:32:12.000000000 +0800
    @@ -180,12 +180,12 @@ int parse_args(const char *name,
    #define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn) \
    int param_set_##name(const char *val, struct kernel_param *kp) \
    { \
    - char *endp; \
    tmptype l; \
    + int ret; \
    \
    if (!val) return -EINVAL; \
    - l = strtolfn(val, &endp, 0); \
    - if (endp == val || ((type)l != l)) \
    + ret = strtolfn(val, 0, &l); \
    + if (ret == -EINVAL || ((type)l != l)) \
    return -EINVAL; \
    *((type *)kp->arg) = l; \
    return 0; \
    @@ -195,13 +195,13 @@ int parse_args(const char *name,
    return sprintf(buffer, format, *((type *)kp->arg)); \
    }

    -STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(short, short, "%hi", long, simple_strtol);
    -STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(int, int, "%i", long, simple_strtol);
    -STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, simple_strtoul);
    -STANDARD_PARAM_DEF(long, long, "%li", long, simple_strtol);
    -STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, simple_strtoul);
    +STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, strict_strtoul);
    +STANDARD_PARAM_DEF(short, short, "%hi", long, strict_strtol);
    +STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, strict_strtoul);
    +STANDARD_PARAM_DEF(int, int, "%i", long, strict_strtol);
    +STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, strict_strtoul);
    +STANDARD_PARAM_DEF(long, long, "%li", long, strict_strtol);
    +STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);

    int param_set_charp(const char *val, struct kernel_param *kp)
    {


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module

    why do we need another kernel cpuid reading method when sched_setaffinity
    exists and cpuid is available in ring3?

    -dean
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast