Re: [PATCH] tty: Fix SIGTTOU not sent with tcflush()

From: Peter Hurley
Date: Thu Sep 26 2013 - 17:18:38 EST


On 09/26/2013 03:10 PM, Oleg Nesterov wrote:
On 09/26, Oleg Nesterov wrote:

Thanks Peter.

Well. I'am afraid my testing was wrong, because Karel reports
it fixes the problem...

But. I applied this patch to my 3.11 tree (last commit is bff157b3a)
which also has the additional patch (03e12617 "tty: disassociate_ctty()
sends the extra SIGCONT"), and

TET_CONFIG=CFG TET_ROOT=. ./T.tcflush 4

still fails... T.tcflush was compiled on another (Karel's) machine,
perhaps there is something in libc, I do not know.

So let me ask just in case, I assume the fix below doesn't depend on
other changes I could miss?

I'll retest after git-pull and report...

Still fails under the Linus's tree + this patch.

However!!!

Tested-by: Oleg Nesterov <oleg@xxxxxxxxxx>

Greg picked this patch up yesterday evening so it went in as-is.


It turns out, T.tcflush doesn't expect it can start as a process group
leader. In this case setsid() fails, then tty_open() doesn't set
signal->tty, and thus this patch makes no difference: tty_check_change()
fails because tty doesn't match signal->tty.

And indeed, this test passes if you run it under strace, or simply do

$ TET_CONFIG=CFG TET_ROOT=. perl -e "system './T.tcflush 4'"

And damn, the fact it doesn't fail under strace doesn't allow you to
see that setsid() fails ;)

I wondered if the test was designed for standalone.

FWIW, I looked at tcflush.c to see what test4() was doing,
wrote my own test vector to confirm the problem, instrumented my
test vector, realized what the problem was, wrote the patch,
built mainline 3.11 + this patch, re-tested the problem, confirmed
the regression went back to 3.10, prepared the patch and sent
the emails.

It's going to take me some time to figure out how best to use LSB;
all the test instrumentation makes the source hard to work with
directly.


This is probably explains why Karel reported success, perhaps he
didn't run this test individually.

Thanks again Peter. Perhaps my analysis was wrong (and I do not see
setsid() in the sources), I do not really care. but perhaps tcflush.c
should be updated.

I would agree with your analysis.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/