Commit 42a359e3 authored by Jeff Dike's avatar Jeff Dike Committed by Linus Torvalds

uml: SIGIO support cleanup

Cleanup of the SIGWINCH support.

Some code and comment reformatting.

The stack used for SIGWINCH threads was leaked.  This is now fixed by storing
it with the pid and other information, and freeing it when the thread is
killed.

If something goes wrong with a WIGWINCH thread, and this is discovered in the
interrupt handler, the winch record would leak.  It is now freed, except that
the IRQ isn't freed.  This is hard to do from interrupt context.  This has the
side-effect that the IRQ system maintains a reference to the freed structure,
but that shouldn't cause a problem since the descriptor is disabled.

register_winch_irq is now much better about cleaning up after an
initialization failure.
Signed-off-by: default avatarJeff Dike <jdike@linux.intel.com>
Cc: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent d14ad81f
...@@ -51,19 +51,21 @@ int generic_console_write(int fd, const char *buf, int n) ...@@ -51,19 +51,21 @@ int generic_console_write(int fd, const char *buf, int n)
/* /*
* UML SIGWINCH handling * UML SIGWINCH handling
* *
* The point of this is to handle SIGWINCH on consoles which have host ttys and * The point of this is to handle SIGWINCH on consoles which have host
* relay them inside UML to whatever might be running on the console and cares * ttys and relay them inside UML to whatever might be running on the
* about the window size (since SIGWINCH notifies about terminal size changes). * console and cares about the window size (since SIGWINCH notifies
* about terminal size changes).
* *
* So, we have a separate thread for each host tty attached to a UML device * So, we have a separate thread for each host tty attached to a UML
* (side-issue - I'm annoyed that one thread can't have multiple controlling * device (side-issue - I'm annoyed that one thread can't have
* ttys for purposed of handling SIGWINCH, but I imagine there are other reasons * multiple controlling ttys for the purpose of handling SIGWINCH, but
* that doesn't make any sense). * I imagine there are other reasons that doesn't make any sense).
* *
* SIGWINCH can't be received synchronously, so you have to set up to receive it * SIGWINCH can't be received synchronously, so you have to set up to
* as a signal. That being the case, if you are going to wait for it, it is * receive it as a signal. That being the case, if you are going to
* convenient to sit in sigsuspend() and wait for the signal to bounce you out of * wait for it, it is convenient to sit in sigsuspend() and wait for
* it (see below for how we make sure to exit only on SIGWINCH). * the signal to bounce you out of it (see below for how we make sure
* to exit only on SIGWINCH).
*/ */
static void winch_handler(int sig) static void winch_handler(int sig)
...@@ -112,7 +114,8 @@ static int winch_thread(void *arg) ...@@ -112,7 +114,8 @@ static int winch_thread(void *arg)
err = os_new_tty_pgrp(pty_fd, os_getpid()); err = os_new_tty_pgrp(pty_fd, os_getpid());
if(err < 0){ if(err < 0){
printk("winch_thread : new_tty_pgrp failed, err = %d\n", -err); printk("winch_thread : new_tty_pgrp failed on fd %d, "
"err = %d\n", pty_fd, -err);
exit(1); exit(1);
} }
...@@ -126,8 +129,9 @@ static int winch_thread(void *arg) ...@@ -126,8 +129,9 @@ static int winch_thread(void *arg)
"err = %d\n", -count); "err = %d\n", -count);
while(1){ while(1){
/* This will be interrupted by SIGWINCH only, since other signals /* This will be interrupted by SIGWINCH only, since
* are blocked.*/ * other signals are blocked.
*/
sigsuspend(&sigs); sigsuspend(&sigs);
count = os_write_file(pipe_fd, &c, sizeof(c)); count = os_write_file(pipe_fd, &c, sizeof(c));
...@@ -137,10 +141,10 @@ static int winch_thread(void *arg) ...@@ -137,10 +141,10 @@ static int winch_thread(void *arg)
} }
} }
static int winch_tramp(int fd, struct tty_struct *tty, int *fd_out) static int winch_tramp(int fd, struct tty_struct *tty, int *fd_out,
unsigned long *stack_out)
{ {
struct winch_data data; struct winch_data data;
unsigned long stack;
int fds[2], n, err; int fds[2], n, err;
char c; char c;
...@@ -153,9 +157,11 @@ static int winch_tramp(int fd, struct tty_struct *tty, int *fd_out) ...@@ -153,9 +157,11 @@ static int winch_tramp(int fd, struct tty_struct *tty, int *fd_out)
data = ((struct winch_data) { .pty_fd = fd, data = ((struct winch_data) { .pty_fd = fd,
.pipe_fd = fds[1] } ); .pipe_fd = fds[1] } );
/* CLONE_FILES so this thread doesn't hold open files which are open /* CLONE_FILES so this thread doesn't hold open files which are open
* now, but later closed. This is a problem with /dev/net/tun. * now, but later closed in a different thread. This is a
* problem with /dev/net/tun, which if held open by this
* thread, prevents the TUN/TAP device from being reused.
*/ */
err = run_helper_thread(winch_thread, &data, CLONE_FILES, &stack, 0); err = run_helper_thread(winch_thread, &data, CLONE_FILES, stack_out, 0);
if(err < 0){ if(err < 0){
printk("fork of winch_thread failed - errno = %d\n", -err); printk("fork of winch_thread failed - errno = %d\n", -err);
goto out_close; goto out_close;
...@@ -187,25 +193,25 @@ static int winch_tramp(int fd, struct tty_struct *tty, int *fd_out) ...@@ -187,25 +193,25 @@ static int winch_tramp(int fd, struct tty_struct *tty, int *fd_out)
void register_winch(int fd, struct tty_struct *tty) void register_winch(int fd, struct tty_struct *tty)
{ {
int pid, thread, thread_fd = -1; unsigned long stack;
int count; int pid, thread, count, thread_fd = -1;
char c = 1; char c = 1;
if(!isatty(fd)) if(!isatty(fd))
return; return;
pid = tcgetpgrp(fd); pid = tcgetpgrp(fd);
if(!CHOOSE_MODE_PROC(is_tracer_winch, is_skas_winch, pid, fd, if (!CHOOSE_MODE_PROC(is_tracer_winch, is_skas_winch, pid, fd, tty) &&
tty) && (pid == -1)){ (pid == -1)) {
thread = winch_tramp(fd, tty, &thread_fd); thread = winch_tramp(fd, tty, &thread_fd, &stack);
if(thread > 0){ if (thread < 0)
register_winch_irq(thread_fd, fd, thread, tty); return;
count = os_write_file(thread_fd, &c, sizeof(c)); register_winch_irq(thread_fd, fd, thread, tty, stack);
if(count != sizeof(c))
printk("register_winch : failed to write " count = os_write_file(thread_fd, &c, sizeof(c));
"synchronization byte, err = %d\n", if(count != sizeof(c))
-count); printk("register_winch : failed to write "
} "synchronization byte, err = %d\n", -count);
} }
} }
...@@ -749,8 +749,24 @@ struct winch { ...@@ -749,8 +749,24 @@ struct winch {
int tty_fd; int tty_fd;
int pid; int pid;
struct tty_struct *tty; struct tty_struct *tty;
unsigned long stack;
}; };
static void free_winch(struct winch *winch, int free_irq_ok)
{
list_del(&winch->list);
if (winch->pid != -1)
os_kill_process(winch->pid, 1);
if (winch->fd != -1)
os_close_file(winch->fd);
if (winch->stack != 0)
free_stack(winch->stack, 0);
if (free_irq_ok)
free_irq(WINCH_IRQ, winch);
kfree(winch);
}
static irqreturn_t winch_interrupt(int irq, void *data) static irqreturn_t winch_interrupt(int irq, void *data)
{ {
struct winch *winch = data; struct winch *winch = data;
...@@ -767,12 +783,13 @@ static irqreturn_t winch_interrupt(int irq, void *data) ...@@ -767,12 +783,13 @@ static irqreturn_t winch_interrupt(int irq, void *data)
"errno = %d\n", -err); "errno = %d\n", -err);
printk("fd %d is losing SIGWINCH support\n", printk("fd %d is losing SIGWINCH support\n",
winch->tty_fd); winch->tty_fd);
free_winch(winch, 0);
return IRQ_HANDLED; return IRQ_HANDLED;
} }
goto out; goto out;
} }
} }
tty = winch->tty; tty = winch->tty;
if (tty != NULL) { if (tty != NULL) {
line = tty->driver_data; line = tty->driver_data;
chan_window_size(&line->chan_list, &tty->winsize.ws_row, chan_window_size(&line->chan_list, &tty->winsize.ws_row,
...@@ -785,43 +802,44 @@ static irqreturn_t winch_interrupt(int irq, void *data) ...@@ -785,43 +802,44 @@ static irqreturn_t winch_interrupt(int irq, void *data)
return IRQ_HANDLED; return IRQ_HANDLED;
} }
void register_winch_irq(int fd, int tty_fd, int pid, struct tty_struct *tty) void register_winch_irq(int fd, int tty_fd, int pid, struct tty_struct *tty,
unsigned long stack)
{ {
struct winch *winch; struct winch *winch;
winch = kmalloc(sizeof(*winch), GFP_KERNEL); winch = kmalloc(sizeof(*winch), GFP_KERNEL);
if (winch == NULL) { if (winch == NULL) {
printk("register_winch_irq - kmalloc failed\n"); printk("register_winch_irq - kmalloc failed\n");
return; goto cleanup;
} }
*winch = ((struct winch) { .list = LIST_HEAD_INIT(winch->list), *winch = ((struct winch) { .list = LIST_HEAD_INIT(winch->list),
.fd = fd, .fd = fd,
.tty_fd = tty_fd, .tty_fd = tty_fd,
.pid = pid, .pid = pid,
.tty = tty }); .tty = tty,
.stack = stack });
if (um_request_irq(WINCH_IRQ, fd, IRQ_READ, winch_interrupt,
IRQF_DISABLED | IRQF_SHARED | IRQF_SAMPLE_RANDOM,
"winch", winch) < 0) {
printk("register_winch_irq - failed to register IRQ\n");
goto out_free;
}
spin_lock(&winch_handler_lock); spin_lock(&winch_handler_lock);
list_add(&winch->list, &winch_handlers); list_add(&winch->list, &winch_handlers);
spin_unlock(&winch_handler_lock); spin_unlock(&winch_handler_lock);
if(um_request_irq(WINCH_IRQ, fd, IRQ_READ, winch_interrupt, return;
IRQF_DISABLED | IRQF_SHARED | IRQF_SAMPLE_RANDOM,
"winch", winch) < 0)
printk("register_winch_irq - failed to register IRQ\n");
}
static void free_winch(struct winch *winch)
{
list_del(&winch->list);
if(winch->pid != -1)
os_kill_process(winch->pid, 1);
if(winch->fd != -1)
os_close_file(winch->fd);
free_irq(WINCH_IRQ, winch); out_free:
kfree(winch); kfree(winch);
cleanup:
os_kill_process(pid, 1);
os_close_file(fd);
if (stack != 0)
free_stack(stack, 0);
} }
static void unregister_winch(struct tty_struct *tty) static void unregister_winch(struct tty_struct *tty)
...@@ -834,7 +852,7 @@ static void unregister_winch(struct tty_struct *tty) ...@@ -834,7 +852,7 @@ static void unregister_winch(struct tty_struct *tty)
list_for_each(ele, &winch_handlers){ list_for_each(ele, &winch_handlers){
winch = list_entry(ele, struct winch, list); winch = list_entry(ele, struct winch, list);
if(winch->tty == tty){ if(winch->tty == tty){
free_winch(winch); free_winch(winch, 1);
break; break;
} }
} }
...@@ -850,7 +868,7 @@ static void winch_cleanup(void) ...@@ -850,7 +868,7 @@ static void winch_cleanup(void)
list_for_each_safe(ele, next, &winch_handlers){ list_for_each_safe(ele, next, &winch_handlers){
winch = list_entry(ele, struct winch, list); winch = list_entry(ele, struct winch, list);
free_winch(winch); free_winch(winch, 1);
} }
spin_unlock(&winch_handler_lock); spin_unlock(&winch_handler_lock);
......
...@@ -42,7 +42,8 @@ extern void generic_free(void *data); ...@@ -42,7 +42,8 @@ extern void generic_free(void *data);
struct tty_struct; struct tty_struct;
extern void register_winch(int fd, struct tty_struct *tty); extern void register_winch(int fd, struct tty_struct *tty);
extern void register_winch_irq(int fd, int tty_fd, int pid, struct tty_struct *tty); extern void register_winch_irq(int fd, int tty_fd, int pid,
struct tty_struct *tty, unsigned long stack);
#define __channel_help(fn, prefix) \ #define __channel_help(fn, prefix) \
__uml_help(fn, prefix "[0-9]*=<channel description>\n" \ __uml_help(fn, prefix "[0-9]*=<channel description>\n" \
......
...@@ -41,7 +41,7 @@ int is_skas_winch(int pid, int fd, void *data) ...@@ -41,7 +41,7 @@ int is_skas_winch(int pid, int fd, void *data)
if(pid != os_getpgrp()) if(pid != os_getpgrp())
return(0); return(0);
register_winch_irq(-1, fd, -1, data); register_winch_irq(-1, fd, -1, data, 0);
return(1); return(1);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment