EINVAL when writing to sysfs with "echo param > file"

[ArenM]

Steps to reproduce:

• run toybox sh
• pick a file in sysfs
• write a valid value to it with echo <value> > <file>

Expected behavior:

The file is written without errors.

Actual behavior:

The command fails, but the file is written.

Hypothesis:

Echo writes the argument passed to it, and the newline at the end in different write calls. This causes the kernel to try to use "\n" as the value in sysfs, which doesn't work.

Here's a snippet from strace:

$ strace toybox sh -c 'echo 0 > brightness'
[snip]
writev(1, [{iov_base="0", iov_len=1}, {iov_base=NULL, iov_len=0}], 2) = 1
writev(1, [{iov_base="", iov_len=0}, {iov_base="\n", iov_len=1}], 2) = -1 EINVAL (Invalid argument)
writev(2, [{iov_base="echo: ", iov_len=6}, {iov_base=NULL, iov_len=0}], 2echo: ) = 6
writev(2, [{iov_base="write", iov_len=5}, {iov_base=NULL, iov_len=0}], 2write) = 5
writev(2, [{iov_base=": Invalid argument", iov_len=18}, {iov_base=NULL, iov_len=0}], 2: Invalid argument) = 18
writev(2, [{iov_base="", iov_len=0}, {iov_base="\n", iov_len=1}], 2
[snip]

But if echo is called with toybox echo it works fine:

$ strace -ff toybox sh -c 'toybox echo 0 > brightness'
[snip]
writev(1, [{iov_base="0", iov_len=1}, {iov_base="\n", iov_len=1}], 2) = 2
[snip]

[landley]

Ah. Standalone echo requests TOYFLAG_LINEBUF but called as a toybox builtin it can't reset the glibc line buffering because "man 3 setlinebuf" says "may be used only after opening a stream and before any other operations have been performed on it". (You'd think if you did a flush you could then change the buffer, but no. I guess it doesn't track whether you passed in a pointer or it allocated its own, so can't free it?)

Anyway, NOFORK is calling echo without line buffering, so the writes are being decoupled. Right. Hmmm... Thanks for the heads up, lemme think about this a bit.

(How on earth do I add a check for this to the regression test suite...)

[landley]

Did commit 3e0e8c6 fix it for you?

I meant to hold off on that until after the next release because I'm going to have missed a flush somewhere, but I've been meaning to do something like that for a while now anyway...

Elliott: heads up, that one's a bit intrusive.

[ArenM]

Yup, it looks like it's fixed in master, thank you.

[enh-google]

Elliott: heads up, that one's a bit intrusive.

thanks. i didn't get time for an update today, but i'll have a look next week. (i think relying more on stdio's defaults is a great choice though, so happy to work through the pain with you on that :-) )

[landley]

I was trying to avoid "oops, I missed a flush" whack-a-mole through the entire codebase, but I guess we're confronting that. I confirmed toysh doesn't use stdio (it deals with raw filehandles because of all the redirection, there's input-side FILE * for line reading but no output to them I'm aware of), and most of the rest I can TOYFLAG_LINEBUF the command.

Another related TODO item I have is turning read_password() into read_rawline() with an extra "echo" parameter, so I don't need to explicitly fflush() before prompt strings that don't end with a newline. But that gets into "readline() supporting unicode" territory, since hexedit is using lib/utf8.c plumbing for that already. There's still some design work here, so for now I just sprayed it down with fflush() and I can grep for that and pull it back out later...

Interactive line editing and command history is the big "toyflag is ready" cliff in the minds of people who don't know the plumbing, so I've been relucant to tackle it with so many known issues still dangling in the code, but at some point I've just gotta push the button...