* Error returns not handled correctly by sysfs.c:subsys_attr_store() @ 2007-11-21 22:16 Andrew Patterson 2007-11-27 4:31 ` Andrew Morton 0 siblings, 1 reply; 16+ messages in thread From: Andrew Patterson @ 2007-11-21 22:16 UTC (permalink / raw) To: linux-kernel; +Cc: linux-hotplug 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-21 22:16 Error returns not handled correctly by sysfs.c:subsys_attr_store() Andrew Patterson @ 2007-11-27 4:31 ` Andrew Morton 2007-11-27 5:33 ` Greg KH 0 siblings, 1 reply; 16+ messages in thread From: Andrew Morton @ 2007-11-27 4:31 UTC (permalink / raw) To: andrew.patterson; +Cc: linux-kernel, linux-hotplug On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <andrew.patterson@hp.com> 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/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-attr.patch So maybe we will soon accidentally fix whatever-this-is? Or maybe we will faithfully maintain it. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-27 4:31 ` Andrew Morton @ 2007-11-27 5:33 ` Greg KH 2007-11-27 5:38 ` Tejun Heo 2007-11-28 7:42 ` Tejun Heo 0 siblings, 2 replies; 16+ messages in thread From: Greg KH @ 2007-11-27 5:33 UTC (permalink / raw) To: Andrew Morton, Tejun Heo; +Cc: andrew.patterson, linux-kernel, linux-hotplug On Mon, Nov 26, 2007 at 08:31:16PM -0800, Andrew Morton wrote: > On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <andrew.patterson@hp.com> 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/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-27 5:33 ` Greg KH @ 2007-11-27 5:38 ` Tejun Heo 2007-11-28 7:42 ` Tejun Heo 1 sibling, 0 replies; 16+ messages in thread From: Tejun Heo @ 2007-11-27 5:38 UTC (permalink / raw) To: Greg KH; +Cc: Andrew Morton, andrew.patterson, linux-kernel, linux-hotplug 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 <andrew.patterson@hp.com> 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/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-27 5:33 ` Greg KH 2007-11-27 5:38 ` Tejun Heo @ 2007-11-28 7:42 ` Tejun Heo 2007-11-28 19:31 ` Andrew Patterson 1 sibling, 1 reply; 16+ messages in thread From: Tejun Heo @ 2007-11-28 7:42 UTC (permalink / raw) To: Greg KH; +Cc: Andrew Morton, andrew.patterson, linux-kernel, linux-hotplug 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 <andrew.patterson@hp.com> 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/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-28 7:42 ` Tejun Heo @ 2007-11-28 19:31 ` Andrew Patterson 2007-11-28 20:05 ` Greg KH 2007-11-29 1:07 ` Tejun Heo 0 siblings, 2 replies; 16+ messages in thread From: Andrew Patterson @ 2007-11-28 19:31 UTC (permalink / raw) To: Tejun Heo; +Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug 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 <andrew.patterson@hp.com> 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/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-28 19:31 ` Andrew Patterson @ 2007-11-28 20:05 ` Greg KH 2007-11-29 1:07 ` Tejun Heo 1 sibling, 0 replies; 16+ messages in thread From: Greg KH @ 2007-11-28 20:05 UTC (permalink / raw) To: Andrew Patterson; +Cc: Tejun Heo, Andrew Morton, linux-kernel, linux-hotplug 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 <andrew.patterson@hp.com> 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/kernel/people/gregkh/gregkh-2.6/gregkh-01-driver/kset-kill-subsys-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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-28 19:31 ` Andrew Patterson 2007-11-28 20:05 ` Greg KH @ 2007-11-29 1:07 ` Tejun Heo 2007-12-03 21:15 ` Andrew Patterson 1 sibling, 1 reply; 16+ messages in thread From: Tejun Heo @ 2007-11-29 1:07 UTC (permalink / raw) To: Andrew Patterson; +Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-11-29 1:07 ` Tejun Heo @ 2007-12-03 21:15 ` Andrew Patterson 2007-12-21 20:35 ` Greg KH 2008-01-03 23:51 ` Andrew Patterson 0 siblings, 2 replies; 16+ messages in thread From: Andrew Patterson @ 2007-12-03 21:15 UTC (permalink / raw) To: Tejun Heo; +Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-12-03 21:15 ` Andrew Patterson @ 2007-12-21 20:35 ` Greg KH 2008-01-03 23:51 ` Andrew Patterson 1 sibling, 0 replies; 16+ messages in thread From: Greg KH @ 2007-12-21 20:35 UTC (permalink / raw) To: Andrew Patterson; +Cc: Tejun Heo, Andrew Morton, linux-kernel, linux-hotplug 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2007-12-03 21:15 ` Andrew Patterson 2007-12-21 20:35 ` Greg KH @ 2008-01-03 23:51 ` Andrew Patterson 2008-01-04 0:07 ` Tejun Heo 1 sibling, 1 reply; 16+ messages in thread From: Andrew Patterson @ 2008-01-03 23:51 UTC (permalink / raw) To: Tejun Heo Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug, bjorn.helgaas 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2008-01-03 23:51 ` Andrew Patterson @ 2008-01-04 0:07 ` Tejun Heo 2008-01-04 0:17 ` Andrew Patterson 0 siblings, 1 reply; 16+ messages in thread From: Tejun Heo @ 2008-01-04 0:07 UTC (permalink / raw) To: Andrew Patterson Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug, bjorn.helgaas 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2008-01-04 0:07 ` Tejun Heo @ 2008-01-04 0:17 ` Andrew Patterson 2008-01-04 0:56 ` Andrew Patterson 2008-01-07 21:13 ` Andrew Patterson 0 siblings, 2 replies; 16+ messages in thread From: Andrew Patterson @ 2008-01-04 0:17 UTC (permalink / raw) To: Tejun Heo Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug, bjorn.helgaas 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2008-01-04 0:17 ` Andrew Patterson @ 2008-01-04 0:56 ` Andrew Patterson 2008-01-04 7:30 ` Andrey Borzenkov 2008-01-07 21:13 ` Andrew Patterson 1 sibling, 1 reply; 16+ messages in thread From: Andrew Patterson @ 2008-01-04 0:56 UTC (permalink / raw) To: Tejun Heo Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug, bjorn.helgaas 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2008-01-04 0:56 ` Andrew Patterson @ 2008-01-04 7:30 ` Andrey Borzenkov 0 siblings, 0 replies; 16+ messages in thread From: Andrey Borzenkov @ 2008-01-04 7:30 UTC (permalink / raw) To: Andrew Patterson Cc: Tejun Heo, Greg KH, Andrew Morton, linux-kernel, linux-hotplug, bjorn.helgaas [-- Attachment #1: Type: text/plain, Size: 5152 bytes --] 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 [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Error returns not handled correctly by sysfs.c:subsys_attr_store() 2008-01-04 0:17 ` Andrew Patterson 2008-01-04 0:56 ` Andrew Patterson @ 2008-01-07 21:13 ` Andrew Patterson 1 sibling, 0 replies; 16+ messages in thread From: Andrew Patterson @ 2008-01-07 21:13 UTC (permalink / raw) To: Tejun Heo Cc: Greg KH, Andrew Morton, linux-kernel, linux-hotplug, bjorn.helgaas 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2008-01-07 21:13 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-11-21 22:16 Error returns not handled correctly by sysfs.c:subsys_attr_store() Andrew Patterson 2007-11-27 4:31 ` Andrew Morton 2007-11-27 5:33 ` Greg KH 2007-11-27 5:38 ` Tejun Heo 2007-11-28 7:42 ` Tejun Heo 2007-11-28 19:31 ` Andrew Patterson 2007-11-28 20:05 ` Greg KH 2007-11-29 1:07 ` Tejun Heo 2007-12-03 21:15 ` Andrew Patterson 2007-12-21 20:35 ` Greg KH 2008-01-03 23:51 ` Andrew Patterson 2008-01-04 0:07 ` Tejun Heo 2008-01-04 0:17 ` Andrew Patterson 2008-01-04 0:56 ` Andrew Patterson 2008-01-04 7:30 ` Andrey Borzenkov 2008-01-07 21:13 ` Andrew Patterson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).