Error returns not handled correctly by sysfs.c:subsys_attr_store() - Kernel

This is a discussion on Error returns not handled correctly by sysfs.c:subsys_attr_store() - Kernel ; The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated correctly when returning a negative value (indicating that an error condition has occurred) is returned. If a negative value is returned, the next subsequent call to subsys_attr_store will have the ...

+ Reply to Thread
Results 1 to 16 of 16

Thread: Error returns not handled correctly by sysfs.c:subsys_attr_store()

  1. Error returns not handled correctly by sysfs.c:subsys_attr_store()

    The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
    correctly when returning a negative value (indicating that an error
    condition has occurred) is returned. If a negative value is returned,
    the next subsequent call to subsys_attr_store will have the contents of
    buf appended to the previous call. Example: I have modified the
    sd.c:sd_store_allow_restart to always print out the contents of the buf
    and return an error using the following patch:

    --- a/drivers/scsi/sd.c
    +++ b/drivers/scsi/sd.c
    @@ -183,6 +183,9 @@ static ssize_t sd_store_allow_restart(struct class_device *c
    struct scsi_disk *sdkp = to_scsi_disk(cdev);
    struct scsi_device *sdp = sdkp->device;

    + printk(KERN_ERR "buf_ptr = 0x%p, buf = %s, count = %u\n", buf, buf, coun
    + return -EINVAL;
    +
    if (!capable(CAP_SYS_ADMIN))
    return -EACCES;

    I get the following output when writing invalid values to the
    allow_restart sysfs file:

    # echo x > /sys/class/scsi_disk/4:0:0:0/allow_restart
    bash: echo: write error: Invalid argument
    # echo y > /sys/class/scsi_disk/4:0:0:0/allow_restart
    bash: echo: write error: Invalid argument
    # echo z > /sys/class/scsi_disk/4:0:0:0/allow_restart
    bash: echo: write error: Invalid argument

    And the console output shows:

    buf_ptr = 0xe00001004bdb0000, buf = x
    , count = 2
    buf_ptr = 0xe00001004bdb0000, buf = x
    , count = 2
    buf_ptr = 0xe00001004bdb0000, buf = x
    y
    , count = 4
    buf_ptr = 0xe00001004bdb0000, buf = x
    y
    , count = 4
    buf_ptr = 0xe00001004caf0000, buf = x
    y
    z
    , count = 6
    buf_ptr = 0xe00001004caf0000, buf = x
    y
    z
    , count = 6

    and the same append problem occurs when using another sysfs file:

    # echo xyzzy > /sys/class/scsi_disk/4:0:1:0/allow_restart
    bash: echo: write error: Invalid argument

    buf_ptr = 0xe00001004caf0000, buf = x
    y
    z
    xyzzy
    , count = 12

    I found this problem in 2.6.24-rc3 and and an earlier version of 2.6.23.

    This seems to work correctly on 2.6.18 (at least with the RHEL5 kernel I
    did some testing with), i.e. the contents of buf from the previous
    failed called are thrown away/overwritten. I looked through sysfs.c to
    see if I could find anything obvious but could not see anything.
    Perhaps this is handled at a higher level.

    --
    Andrew Patterson
    Hewlett-Packard

    -
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson wrote:

    > The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
    > correctly when returning a negative value (indicating that an error
    > condition has occurred) is returned. If a negative value is returned,
    > the next subsequent call to subsys_attr_store will have the contents of
    > buf appended to the previous call.


    subsys_attr_store() gets deleted by
    http://www.kernel.org/pub/linux/kern...sys-attr.patch

    So maybe we will soon accidentally fix whatever-this-is? Or maybe we will
    faithfully maintain it.
    -
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
    > On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson wrote:
    >
    > > The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
    > > correctly when returning a negative value (indicating that an error
    > > condition has occurred) is returned. If a negative value is returned,
    > > the next subsequent call to subsys_attr_store will have the contents of
    > > buf appended to the previous call.

    >
    > subsys_attr_store() gets deleted by
    > http://www.kernel.org/pub/linux/kern...sys-attr.patch
    >
    > So maybe we will soon accidentally fix whatever-this-is? Or maybe we will
    > faithfully maintain it.


    Yes, subsys attributes go away, but this is showing a bug in the sysfs
    core with attributes, not in the "middle" layers of attributes.

    I bounced the original bug report to Tejun, who has been changing the
    logic around this area to see if he sees anything that might be
    different now.

    Tejun?

    thanks,

    greg k-h
    -
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    Greg KH wrote:
    > On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
    >> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson wrote:
    >>
    >>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
    >>> correctly when returning a negative value (indicating that an error
    >>> condition has occurred) is returned. If a negative value is returned,
    >>> the next subsequent call to subsys_attr_store will have the contents of
    >>> buf appended to the previous call.

    >> subsys_attr_store() gets deleted by
    >> http://www.kernel.org/pub/linux/kern...sys-attr.patch
    >>
    >> So maybe we will soon accidentally fix whatever-this-is? Or maybe we will
    >> faithfully maintain it.

    >
    > Yes, subsys attributes go away, but this is showing a bug in the sysfs
    > core with attributes, not in the "middle" layers of attributes.
    >
    > I bounced the original bug report to Tejun, who has been changing the
    > logic around this area to see if he sees anything that might be
    > different now.
    >
    > Tejun?


    (groaning buried under ATA bugs) Will take a look soon.

    Thanks.

    --
    tejun
    -
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    Greg KH wrote:
    > On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
    >> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson wrote:
    >>
    >>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
    >>> correctly when returning a negative value (indicating that an error
    >>> condition has occurred) is returned. If a negative value is returned,
    >>> the next subsequent call to subsys_attr_store will have the contents of
    >>> buf appended to the previous call.

    >> subsys_attr_store() gets deleted by
    >> http://www.kernel.org/pub/linux/kern...sys-attr.patch
    >>
    >> So maybe we will soon accidentally fix whatever-this-is? Or maybe we will
    >> faithfully maintain it.

    >
    > Yes, subsys attributes go away, but this is showing a bug in the sysfs
    > core with attributes, not in the "middle" layers of attributes.
    >
    > I bounced the original bug report to Tejun, who has been changing the
    > logic around this area to see if he sees anything that might be
    > different now.
    >
    > Tejun?


    Weird, the problem is not reproducible here.

    # echo a > allow_restart
    -bash: echo: write error: Invalid argument
    [ 437.518024] buf_ptr = 0xffff810005e20000, buf = x
    [ 437.518027] , count = 2
    # echo b > allow_restart
    -bash: echo: write error: Invalid argument
    [ 438.972973] buf_ptr = 0xffff81001be6f000, buf = y
    [ 438.972976] , count = 2
    # echo c > allow_restart
    -bash: echo: write error: Invalid argument
    [ 440.539747] buf_ptr = 0xffff81001d4ba000, buf = z
    [ 440.539750] , count = 2

    Which is expected. On each open, sysfs_buffer is allocated with kzalloc
    and the buffer is freed on close, so I don't see how it can happen.
    Behavior for multiple write can be considered peculiar in that ppos is
    essentially ignored and each write is passed just like brand new write
    to ->store method but this too is the expected behavior.

    # (echo a; echo b; echo c) > allow_restart
    [ 765.257132] buf_ptr = 0xffff81001be4f000, buf = a
    [ 765.257135] , count = 2
    [ 765.285474] buf_ptr = 0xffff81001be4f000, buf = b
    [ 765.285484] , count = 2
    [ 765.314002] buf_ptr = 0xffff81001be4f000, buf = c
    [ 765.314004] , count = 2
    -bash: echo: write error: Invalid argument
    -bash: echo: write error: Invalid argument
    -bash: echo: write error: Invalid argument

    Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
    and retry?

    --
    tejun
    -
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Wed, 2007-11-28 at 16:42 +0900, Tejun Heo wrote:
    > Greg KH wrote:
    > > On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
    > >> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson wrote:
    > >>
    > >>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
    > >>> correctly when returning a negative value (indicating that an error
    > >>> condition has occurred) is returned. If a negative value is returned,
    > >>> the next subsequent call to subsys_attr_store will have the contents of
    > >>> buf appended to the previous call.
    > >> subsys_attr_store() gets deleted by
    > >> http://www.kernel.org/pub/linux/kern...sys-attr.patch
    > >>
    > >> So maybe we will soon accidentally fix whatever-this-is? Or maybe we will
    > >> faithfully maintain it.

    > >
    > > Yes, subsys attributes go away, but this is showing a bug in the sysfs
    > > core with attributes, not in the "middle" layers of attributes.
    > >
    > > I bounced the original bug report to Tejun, who has been changing the
    > > logic around this area to see if he sees anything that might be
    > > different now.
    > >
    > > Tejun?

    >
    > Weird, the problem is not reproducible here.
    >
    > # echo a > allow_restart
    > -bash: echo: write error: Invalid argument
    > [ 437.518024] buf_ptr = 0xffff810005e20000, buf = x
    > [ 437.518027] , count = 2
    > # echo b > allow_restart
    > -bash: echo: write error: Invalid argument
    > [ 438.972973] buf_ptr = 0xffff81001be6f000, buf = y
    > [ 438.972976] , count = 2
    > # echo c > allow_restart
    > -bash: echo: write error: Invalid argument
    > [ 440.539747] buf_ptr = 0xffff81001d4ba000, buf = z
    > [ 440.539750] , count = 2
    >
    > Which is expected. On each open, sysfs_buffer is allocated with kzalloc
    > and the buffer is freed on close, so I don't see how it can happen.
    > Behavior for multiple write can be considered peculiar in that ppos is
    > essentially ignored and each write is passed just like brand new write
    > to ->store method but this too is the expected behavior.
    >
    > # (echo a; echo b; echo c) > allow_restart
    > [ 765.257132] buf_ptr = 0xffff81001be4f000, buf = a
    > [ 765.257135] , count = 2
    > [ 765.285474] buf_ptr = 0xffff81001be4f000, buf = b
    > [ 765.285484] , count = 2
    > [ 765.314002] buf_ptr = 0xffff81001be4f000, buf = c
    > [ 765.314004] , count = 2
    > -bash: echo: write error: Invalid argument
    > -bash: echo: write error: Invalid argument
    > -bash: echo: write error: Invalid argument
    >
    > Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
    > and retry?
    >


    I tried with clean 2.6.24-rc3 and get the same bad behavior. This is on
    an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
    Oh, one other thing. I tried a "uname -r" to make sure I had the
    correct kernel booted and got:

    # uname -r
    2.6.24-rc3
    x
    y
    z
    #

    Andrew
    --
    Andrew Patterson
    Hewlett-Packard

    -
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Wed, Nov 28, 2007 at 12:31:40PM -0700, Andrew Patterson wrote:
    > On Wed, 2007-11-28 at 16:42 +0900, Tejun Heo wrote:
    > > Greg KH wrote:
    > > > On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote:
    > > >> On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson wrote:
    > > >>
    > > >>> The buf in fs/sysfs.c:subsys_attr_store() does not seem to be updated
    > > >>> correctly when returning a negative value (indicating that an error
    > > >>> condition has occurred) is returned. If a negative value is returned,
    > > >>> the next subsequent call to subsys_attr_store will have the contents of
    > > >>> buf appended to the previous call.
    > > >> subsys_attr_store() gets deleted by
    > > >> http://www.kernel.org/pub/linux/kern...sys-attr.patch
    > > >>
    > > >> So maybe we will soon accidentally fix whatever-this-is? Or maybe we will
    > > >> faithfully maintain it.
    > > >
    > > > Yes, subsys attributes go away, but this is showing a bug in the sysfs
    > > > core with attributes, not in the "middle" layers of attributes.
    > > >
    > > > I bounced the original bug report to Tejun, who has been changing the
    > > > logic around this area to see if he sees anything that might be
    > > > different now.
    > > >
    > > > Tejun?

    > >
    > > Weird, the problem is not reproducible here.
    > >
    > > # echo a > allow_restart
    > > -bash: echo: write error: Invalid argument
    > > [ 437.518024] buf_ptr = 0xffff810005e20000, buf = x
    > > [ 437.518027] , count = 2
    > > # echo b > allow_restart
    > > -bash: echo: write error: Invalid argument
    > > [ 438.972973] buf_ptr = 0xffff81001be6f000, buf = y
    > > [ 438.972976] , count = 2
    > > # echo c > allow_restart
    > > -bash: echo: write error: Invalid argument
    > > [ 440.539747] buf_ptr = 0xffff81001d4ba000, buf = z
    > > [ 440.539750] , count = 2
    > >
    > > Which is expected. On each open, sysfs_buffer is allocated with kzalloc
    > > and the buffer is freed on close, so I don't see how it can happen.
    > > Behavior for multiple write can be considered peculiar in that ppos is
    > > essentially ignored and each write is passed just like brand new write
    > > to ->store method but this too is the expected behavior.
    > >
    > > # (echo a; echo b; echo c) > allow_restart
    > > [ 765.257132] buf_ptr = 0xffff81001be4f000, buf = a
    > > [ 765.257135] , count = 2
    > > [ 765.285474] buf_ptr = 0xffff81001be4f000, buf = b
    > > [ 765.285484] , count = 2
    > > [ 765.314002] buf_ptr = 0xffff81001be4f000, buf = c
    > > [ 765.314004] , count = 2
    > > -bash: echo: write error: Invalid argument
    > > -bash: echo: write error: Invalid argument
    > > -bash: echo: write error: Invalid argument
    > >
    > > Andrew Petterson, can you please build 2.6.24-rc3 from clean source tree
    > > and retry?
    > >

    >
    > I tried with clean 2.6.24-rc3 and get the same bad behavior. This is on
    > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.


    Please do so.

    > Oh, one other thing. I tried a "uname -r" to make sure I had the
    > correct kernel booted and got:
    >
    > # uname -r
    > 2.6.24-rc3
    > x
    > y
    > z


    Heh, that's not good, try a clean tree

    thanks,

    greg k-h
    -
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    Andrew Patterson wrote:
    > I tried with clean 2.6.24-rc3 and get the same bad behavior. This is on
    > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
    > Oh, one other thing. I tried a "uname -r" to make sure I had the
    > correct kernel booted and got:
    >
    > # uname -r
    > 2.6.24-rc3
    > x
    > y
    > z
    > #


    Yeah, please try it on another machine from clean tree. sysfs code is
    definitely not endian dependent and is 64 bit clean. Heck, all my test
    machines run 64 bit these days. I would be surprised if it's something
    architecture dependent but please try on a different machine with
    different userland with kernel built from fresh source tree.

    Thanks.

    --
    tejun
    -
    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. Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
    > Andrew Patterson wrote:
    > > I tried with clean 2.6.24-rc3 and get the same bad behavior. This is on
    > > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
    > > Oh, one other thing. I tried a "uname -r" to make sure I had the
    > > correct kernel booted and got:
    > >
    > > # uname -r
    > > 2.6.24-rc3
    > > x
    > > y
    > > z
    > > #

    >
    > Yeah, please try it on another machine from clean tree. sysfs code is
    > definitely not endian dependent and is 64 bit clean. Heck, all my test
    > machines run 64 bit these days. I would be surprised if it's something
    > architecture dependent but please try on a different machine with
    > different userland with kernel built from fresh source tree.
    >
    > Thanks.


    I tried this on a AMD system running an i386 kernel. I get the same bad
    behavior. This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
    I ran "make mrproper" followed by "make oldconfig" and accepted all the
    defaults for the config.

    There is one slight change with this experiment. Other nodes are not
    getting corrupted, i.e., uname -r is getting the correct value.

    --
    Andrew Patterson
    Hewlett-Packard Company

    --
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Mon, Dec 03, 2007 at 02:15:58PM -0700, Andrew Patterson wrote:
    > On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
    > > Andrew Patterson wrote:
    > > > I tried with clean 2.6.24-rc3 and get the same bad behavior. This is on
    > > > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
    > > > Oh, one other thing. I tried a "uname -r" to make sure I had the
    > > > correct kernel booted and got:
    > > >
    > > > # uname -r
    > > > 2.6.24-rc3
    > > > x
    > > > y
    > > > z
    > > > #

    > >
    > > Yeah, please try it on another machine from clean tree. sysfs code is
    > > definitely not endian dependent and is 64 bit clean. Heck, all my test
    > > machines run 64 bit these days. I would be surprised if it's something
    > > architecture dependent but please try on a different machine with
    > > different userland with kernel built from fresh source tree.
    > >
    > > Thanks.

    >
    > I tried this on a AMD system running an i386 kernel. I get the same bad
    > behavior. This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
    > I ran "make mrproper" followed by "make oldconfig" and accepted all the
    > defaults for the config.
    >
    > There is one slight change with this experiment. Other nodes are not
    > getting corrupted, i.e., uname -r is getting the correct value.


    Are you still seeing this on 2.6.24-rc6?

    I still can not duplicate this here

    thanks,

    greg k-h
    --
    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. Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Mon, 2007-12-03 at 14:15 -0700, Andrew Patterson wrote:
    > On Thu, 2007-11-29 at 10:07 +0900, Tejun Heo wrote:
    > > Andrew Patterson wrote:
    > > > I tried with clean 2.6.24-rc3 and get the same bad behavior. This is on
    > > > an ia64 box, so maybe that is an issue. I can try on an x86 box as well.
    > > > Oh, one other thing. I tried a "uname -r" to make sure I had the
    > > > correct kernel booted and got:
    > > >
    > > > # uname -r
    > > > 2.6.24-rc3
    > > > x
    > > > y
    > > > z
    > > > #

    > >
    > > Yeah, please try it on another machine from clean tree. sysfs code is
    > > definitely not endian dependent and is 64 bit clean. Heck, all my test
    > > machines run 64 bit these days. I would be surprised if it's something
    > > architecture dependent but please try on a different machine with
    > > different userland with kernel built from fresh source tree.
    > >
    > > Thanks.

    >
    > I tried this on a AMD system running an i386 kernel. I get the same bad
    > behavior. This is from a 2.6.24-rc3 kernel downloaded from kernel.org.
    > I ran "make mrproper" followed by "make oldconfig" and accepted all the
    > defaults for the config.
    >
    > There is one slight change with this experiment. Other nodes are not
    > getting corrupted, i.e., uname -r is getting the correct value.


    It looks like this is a shell issue. After looking through the sysfs
    code, I realized that this problem seems to be driven from user-land.
    So I performed some experiments:

    1. Wrote a simple program that just used write(2) to write to the
    sysfs entry. This works fine.
    2. Used /bin/echo instead of the built-in echo command. This too
    works fine.
    3. Tried several shells. Zsh and Bash both fail. Csh works fine.

    I then ran strace on the following shell-script:

    #!/bin/bash

    echo x > allow_restart
    echo y > allow_restart
    echo z > allow_restart

    and got:

    # strace -e trace=write ~/tmp/tester.sh
    write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
    ) = 72
    write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
    ) = 72
    write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
    ) = 72
    write(1, "x\ny\nz\n", 6x
    y
    z
    ) = 6
    Process 3800 detached

    As you can see, subsequent echo commands have their arguments appended
    to the previous command. So it seems that the shell is not resetting the
    buffer between failed echo commands.

    --
    Andrew Patterson
    Hewlett-Packard Company

    --
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    Hello,

    Andrew Patterson wrote:
    > It looks like this is a shell issue. After looking through the sysfs
    > code, I realized that this problem seems to be driven from user-land.
    > So I performed some experiments:
    >
    > 1. Wrote a simple program that just used write(2) to write to the
    > sysfs entry. This works fine.
    > 2. Used /bin/echo instead of the built-in echo command. This too
    > works fine.
    > 3. Tried several shells. Zsh and Bash both fail. Csh works fine.
    >
    > I then ran strace on the following shell-script:
    >
    > #!/bin/bash
    >
    > echo x > allow_restart
    > echo y > allow_restart
    > echo z > allow_restart
    >
    > and got:
    >
    > # strace -e trace=write ~/tmp/tester.sh
    > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
    > ) = 72
    > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
    > ) = 72
    > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
    > ) = 72
    > write(1, "x\ny\nz\n", 6x
    > y
    > z
    > ) = 6
    > Process 3800 detached


    Eeeeeeeekkkk.... That's scary. Which distro are you using and what does
    'bash --version' say?

    --
    tejun
    --
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()


    On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
    > Hello,
    >
    > Andrew Patterson wrote:
    > > It looks like this is a shell issue. After looking through the sysfs
    > > code, I realized that this problem seems to be driven from user-land.
    > > So I performed some experiments:
    > >
    > > 1. Wrote a simple program that just used write(2) to write to the
    > > sysfs entry. This works fine.
    > > 2. Used /bin/echo instead of the built-in echo command. This too
    > > works fine.
    > > 3. Tried several shells. Zsh and Bash both fail. Csh works fine.
    > >
    > > I then ran strace on the following shell-script:
    > >
    > > #!/bin/bash
    > >
    > > echo x > allow_restart
    > > echo y > allow_restart
    > > echo z > allow_restart
    > >
    > > and got:
    > >
    > > # strace -e trace=write ~/tmp/tester.sh
    > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
    > > ) = 72
    > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
    > > ) = 72
    > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
    > > ) = 72
    > > write(1, "x\ny\nz\n", 6x
    > > y
    > > z
    > > ) = 6
    > > Process 3800 detached

    >
    > Eeeeeeeekkkk.... That's scary. Which distro are you using and what does
    > 'bash --version' say?


    IA64 Debian lenny.

    # bash --version
    GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)

    # zsh --version
    zsh 4.3.4 (ia64-unknown-linux-gnu)

    # csh --version
    tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
    wide,nls,dl,al,kan,rh,nd,color,filec

    I suppose I should try this an ia32 box again, and perhaps with some
    other distros. I am not sure what the kernel can do about this, but it
    might be nice to report it to the shell maintainers.

    --
    Andrew Patterson
    Hewlett-Packard Company

    --
    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: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
    > On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
    > > Hello,
    > >
    > > Andrew Patterson wrote:
    > > > It looks like this is a shell issue. After looking through the sysfs
    > > > code, I realized that this problem seems to be driven from user-land.
    > > > So I performed some experiments:
    > > >
    > > > 1. Wrote a simple program that just used write(2) to write to the
    > > > sysfs entry. This works fine.
    > > > 2. Used /bin/echo instead of the built-in echo command. This too
    > > > works fine.
    > > > 3. Tried several shells. Zsh and Bash both fail. Csh works fine.
    > > >
    > > > I then ran strace on the following shell-script:
    > > >
    > > > #!/bin/bash
    > > >
    > > > echo x > allow_restart
    > > > echo y > allow_restart
    > > > echo z > allow_restart
    > > >
    > > > and got:
    > > >
    > > > # strace -e trace=write ~/tmp/tester.sh
    > > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
    > > > ) = 72
    > > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
    > > > ) = 72
    > > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
    > > > ) = 72
    > > > write(1, "x\ny\nz\n", 6x
    > > > y
    > > > z
    > > > ) = 6
    > > > Process 3800 detached

    > >
    > > Eeeeeeeekkkk.... That's scary. Which distro are you using and what does
    > > 'bash --version' say?

    >
    > IA64 Debian lenny.
    >
    > # bash --version
    > GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
    >
    > # zsh --version
    > zsh 4.3.4 (ia64-unknown-linux-gnu)
    >
    > # csh --version
    > tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
    > wide,nls,dl,al,kan,rh,nd,color,filec
    >
    > I suppose I should try this an ia32 box again, and perhaps with some
    > other distros. I am not sure what the kernel can do about this, but it
    > might be nice to report it to the shell maintainers.


    Some further tests:

    AMD running Debian lenny with i686 kernel -- fails.
    Bash version = 3.1.17(1)

    Intel running Ubuntu/gutsy with i686 kernel -- fails.
    Bash version = 3.2.25(1)

    Itanium running SLES10 with ia64 kernel -- succeeds.
    Bash version = 3.1.17(1)

    BTW, I found a way to reproduce this without modifying the kernel.
    The /sys/class/scsi_host/*/state sysfs store routine returns EINVAL if
    an invalid state is written. So just echo 2 bad values to the the state
    sysfs entry while running strace.

    Andrew

    --
    Andrew Patterson
    Hewlett-Packard Company

    --
    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. Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Friday 04 January 2008, Andrew Patterson wrote:
    > On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
    > > On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
    > > > Hello,
    > > >
    > > > Andrew Patterson wrote:
    > > > > It looks like this is a shell issue. After looking through the sysfs
    > > > > code, I realized that this problem seems to be driven from user-land.
    > > > > So I performed some experiments:
    > > > >
    > > > > 1. Wrote a simple program that just used write(2) to write to the
    > > > > sysfs entry. This works fine.
    > > > > 2. Used /bin/echo instead of the built-in echo command. This too
    > > > > works fine.
    > > > > 3. Tried several shells. Zsh and Bash both fail. Csh works fine.
    > > > >
    > > > > I then ran strace on the following shell-script:
    > > > >
    > > > > #!/bin/bash
    > > > >
    > > > > echo x > allow_restart
    > > > > echo y > allow_restart
    > > > > echo z > allow_restart
    > > > >
    > > > > and got:
    > > > >
    > > > > # strace -e trace=write ~/tmp/tester.sh
    > > > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > > > write(2, "/home/andrew/tmp/tester.sh: line"...,

    72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
    > > > > ) = 72
    > > > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > > > write(2, "/home/andrew/tmp/tester.sh: line"...,

    72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
    > > > > ) = 72
    > > > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > > > write(2, "/home/andrew/tmp/tester.sh: line"...,

    72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
    > > > > ) = 72
    > > > > write(1, "x\ny\nz\n", 6x
    > > > > y
    > > > > z
    > > > > ) = 6
    > > > > Process 3800 detached
    > > >
    > > > Eeeeeeeekkkk.... That's scary. Which distro are you using and what does
    > > > 'bash --version' say?

    > >
    > > IA64 Debian lenny.
    > >
    > > # bash --version
    > > GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
    > >
    > > # zsh --version
    > > zsh 4.3.4 (ia64-unknown-linux-gnu)
    > >
    > > # csh --version
    > > tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
    > > wide,nls,dl,al,kan,rh,nd,color,filec
    > >
    > > I suppose I should try this an ia32 box again, and perhaps with some
    > > other distros. I am not sure what the kernel can do about this, but it
    > > might be nice to report it to the shell maintainers.

    >
    > Some further tests:
    >
    > AMD running Debian lenny with i686 kernel -- fails.
    > Bash version = 3.1.17(1)
    >
    > Intel running Ubuntu/gutsy with i686 kernel -- fails.
    > Bash version = 3.2.25(1)
    >
    > Itanium running SLES10 with ia64 kernel -- succeeds.
    > Bash version = 3.1.17(1)
    >
    > BTW, I found a way to reproduce this without modifying the kernel.
    > The /sys/class/scsi_host/*/state sysfs store routine returns EINVAL if
    > an invalid state is written. So just echo 2 bad values to the the state
    > sysfs entry while running strace.
    >


    I can't reproduce it using zsh either 4.3.4 as shipped by Mandriva or zsh CVS
    head. In both cases it echoes correct argument. Nor do I see double writes's in
    strace.

    {pts/0}% sudo strace -e trace=write /tmp/foo # zsh 4.3.4
    write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    write(2, "/tmp/foo:echo:3: write error: \320\235"..., 72/tmp/foo:echo:3: write
    error: Недопустимый аргумент
    ) = 72
    write(1, "y\n", 2) = -1 EINVAL (Invalid argument)
    write(2, "/tmp/foo:echo:4: write error: \320\235"..., 72/tmp/foo:echo:4: write
    error: Недопустимый аргумент
    ) = 72
    write(1, "z\n", 2) = -1 EINVAL (Invalid argument)
    write(2, "/tmp/foo:echo:5: write error: \320\235"..., 72/tmp/foo:echo:5: write
    error: Недопустимый аргумент
    ) = 72
    {pts/0}% sudo strace -e trace=write /tmp/foo # zsh CVS head
    write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    write(2, "/tmp/foo:echo:3: write error: \320\235"..., 72/tmp/foo:echo:3: write
    error: Недопустимый аргумент
    ) = 72
    write(1, "y\n", 2) = -1 EINVAL (Invalid argument)
    write(2, "/tmp/foo:echo:4: write error: \320\235"..., 72/tmp/foo:echo:4: write
    error: Недопустимый аргумент
    ) = 72
    write(1, "z\n", 2) = -1 EINVAL (Invalid argument)
    write(2, "/tmp/foo:echo:5: write error: \320\235"..., 72/tmp/foo:echo:5: write
    error: Недопустимый аргумент
    ) = 72

    {pts/0}% cat /tmp/foo
    #!/home/bor/pkg/bin/zsh -f

    echo x > state
    echo y > state
    echo z > state

    where state is /sys/power/state


    {pts/1}% zsh --version
    zsh 4.3.4 (i586-mandriva-linux-gnu)
    {pts/1}% ~/pkg/bin/zsh --version
    zsh 4.3.4-dev-6 (i686-pc-linux-gnu)

    -andrey

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.8 (GNU/Linux)

    iEYEABECAAYFAkd94J0ACgkQR6LMutpd94wT3wCgq24S2R4GSo WkMxalzjCAqmjY
    umAAnj0Q+BKqIabHYyA9gMcW/IbSxnj6
    =uD7h
    -----END PGP SIGNATURE-----


  16. Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()

    On Thu, 2008-01-03 at 17:17 -0700, Andrew Patterson wrote:
    > On Fri, 2008-01-04 at 09:07 +0900, Tejun Heo wrote:
    > > Hello,
    > >
    > > Andrew Patterson wrote:
    > > > It looks like this is a shell issue. After looking through the sysfs
    > > > code, I realized that this problem seems to be driven from user-land.
    > > > So I performed some experiments:
    > > >
    > > > 1. Wrote a simple program that just used write(2) to write to the
    > > > sysfs entry. This works fine.
    > > > 2. Used /bin/echo instead of the built-in echo command. This too
    > > > works fine.
    > > > 3. Tried several shells. Zsh and Bash both fail. Csh works fine.
    > > >
    > > > I then ran strace on the following shell-script:
    > > >
    > > > #!/bin/bash
    > > >
    > > > echo x > allow_restart
    > > > echo y > allow_restart
    > > > echo z > allow_restart
    > > >
    > > > and got:
    > > >
    > > > # strace -e trace=write ~/tmp/tester.sh
    > > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > > write(1, "x\n", 2) = -1 EINVAL (Invalid argument)
    > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 4: echo: write error: Invalid argument
    > > > ) = 72
    > > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > > write(1, "x\ny\n", 4) = -1 EINVAL (Invalid argument)
    > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 5: echo: write error: Invalid argument
    > > > ) = 72
    > > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > > write(1, "x\ny\nz\n", 6) = -1 EINVAL (Invalid argument)
    > > > write(2, "/home/andrew/tmp/tester.sh: line"..., 72/home/andrew/tmp/tester.sh: line 6: echo: write error: Invalid argument
    > > > ) = 72
    > > > write(1, "x\ny\nz\n", 6x
    > > > y
    > > > z
    > > > ) = 6
    > > > Process 3800 detached

    > >
    > > Eeeeeeeekkkk.... That's scary. Which distro are you using and what does
    > > 'bash --version' say?

    >
    > IA64 Debian lenny.
    >
    > # bash --version
    > GNU bash, version 3.1.17(1)-release (ia64-unknown-linux-gnu)
    >
    > # zsh --version
    > zsh 4.3.4 (ia64-unknown-linux-gnu)
    >
    > # csh --version
    > tcsh 6.14.00 (Astron) 2005-03-25 (ia64-unknown-linux) options
    > wide,nls,dl,al,kan,rh,nd,color,filec
    >
    > I suppose I should try this an ia32 box again, and perhaps with some
    > other distros. I am not sure what the kernel can do about this, but it
    > might be nice to report it to the shell maintainers.
    >


    This looks like it might be the culprit.

    http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=459643

    The fact that it works on SLES10 lends further evidence to glibc being
    the problem.

    --
    Andrew Patterson
    Hewlett-Packard Company

    --
    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