[Bug 3547] shell read is maybe too safe

Denys Vlasenko vda.linux at googlemail.com
Sun May 8 23:20:16 UTC 2011


On Monday 25 April 2011 23:35, Ian Wienand wrote:
> Hi Denys,
> 
> Did you have any further thoughts on this?

Fixed in git.

Although I think the fix revealed a deepere problem:
hush's way of handling signals isn't good for read
and for reading commands from stdin: since we block
signals we are interested in, we cannot detect them
while we wait for input on a fd.

Changing signal handling to a different scheme,
where they are caught and recorded, will somewhat help,
because now poll/read syscalls will be interruptible,
but this creates another problem: many other sycalls
may start failing with EINTR too, and we don't check
for that. The problem is, EINTR case happens rarely.
IOW: we may have many rarely-triggered bugs, one
per every syscall we don't wrap with "repeat on EINTR"
code, or with "if we got EINTR, propagate EINTR".

Current fix in hush enables such handling _only_ for read -
see the "dance" we do around shell_builtin_read()
in builtin_read().

For the possible problems it introduced, think about this:
if we use "read -p Prompt:", we execute this
in shell_builtin_read():

        if (opt_p && isatty(fd)) {
                fputs(opt_p, stderr);
                fflush_all();
        }

What will happen if signal arrives EXACTLY at the moment
write() inside fputs() is executed?

We'll got EINTR. We will ignore it, go into
nonblock_immune_read() -> read(), and start sleeping
on read syscall.

Result: read builtin will fail to be interrupted by a signal.
Bug. Difficult-to-reproduce bug.

Ideas anyone?

-- 
vda


More information about the busybox mailing list