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 > > > > Eeee 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 > > > > > > Eeee 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 signature.asc Description: This is a digitally signed message part.
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 > > > > Eeee 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 > > Eeee 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 Eeee 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 <[EMAIL PROTECTED]> > > >> 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 = 0x810005e2, buf = x > > [ 437.518027] , count = 2 > > # echo b > allow_restart > > -bash: echo: write error: Invalid argument > > [ 438.972973] buf_ptr = 0x81001be6f000, buf = y > > [ 438.972976] , count = 2 > > # echo c > allow_restart > > -bash: echo: write error: Invalid argument > > [ 440.539747] buf_ptr = 0x81001d4ba000, 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 = 0x81001be4f000, buf = a > > [ 765.257135] , count = 2 > > [ 765.285474] buf_ptr = 0x81001be4f000, buf = b > > [ 765.285484] , count = 2 > > [ 765.314002] buf_ptr = 0x81001be4f000, 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 <[EMAIL PROTECTED]> > >> 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 = 0x810005e2, buf = x > [ 437.518027] , count = 2 > # echo b > allow_restart > -bash: echo: write error: Invalid argument > [ 438.972973] buf_ptr = 0x81001be6f000, buf = y > [ 438.972976] , count = 2 > # echo c > allow_restart > -bash: echo: write error: Invalid argument > [ 440.539747] buf_ptr = 0x81001d4ba000, 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 = 0x81001be4f000, buf = a > [ 765.257135] , count = 2 > [ 765.285474] buf_ptr = 0x81001be4f000, buf = b > [ 765.285484] , count = 2 > [ 765.314002] buf_ptr = 0x81001be4f000, 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 <[EMAIL PROTECTED]> >> 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 = 0x810005e2, buf = x [ 437.518027] , count = 2 # echo b > allow_restart -bash: echo: write error: Invalid argument [ 438.972973] buf_ptr = 0x81001be6f000, buf = y [ 438.972976] , count = 2 # echo c > allow_restart -bash: echo: write error: Invalid argument [ 440.539747] buf_ptr = 0x81001d4ba000, 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 = 0x81001be4f000, buf = a [ 765.257135] , count = 2 [ 765.285474] buf_ptr = 0x81001be4f000, buf = b [ 765.285484] , count = 2 [ 765.314002] buf_ptr = 0x81001be4f000, 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 <[EMAIL PROTECTED]> >> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 <[EMAIL PROTECTED]> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Error returns not handled correctly by sysfs.c:subsys_attr_store()
On Wed, 21 Nov 2007 15:16:59 -0700 Andrew Patterson <[EMAIL PROTECTED]> 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 = 0xe1004bdb, buf = x , count = 2 buf_ptr = 0xe1004bdb, buf = x , count = 2 buf_ptr = 0xe1004bdb, buf = x y , count = 4 buf_ptr = 0xe1004bdb, buf = x y , count = 4 buf_ptr = 0xe1004caf, buf = x y z , count = 6 buf_ptr = 0xe1004caf, 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 = 0xe1004caf, 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/