Bug#1005900: linux.uml: uml_mconsole client becomes blocked on read after issuing commands stop and go

[Anton Ivanov]

Hi Ritesh, hi Mihai,

Apologies for the delay in the answer, I was traveling last week.

1. Your patch will not achieve the desired aim. The IRQS in UML nowadays are per fd and looping inside the IRQ handler for mconsole will not stop UML as it used to when it was using the old poll() based IRQ subsystem. It will still handle other IRQs. That is a bug, we need to see what can be done here.

2. Otherwise, just a - on the deactivate_fd() would do the trick. There is a reentrancy check on the IRQ handler and while you are looping inside it, the same IRQ will not be triggered again. No need to deactivate_fd(). However, as per "1", this is insufficient - all other IRQS will still be handled.

Brgds,

A.

On 18/02/2022 08:38, Ritesh Raj Sarraf wrote:
> Hello Mihai,
> 
> In this case, it is good to have the User Mode Linux upstream in the
> loop.
> 
> Thanks,
> Ritesh
> 
> --- linux-source-5.16/arch/um/drivers/mconsole_kern.c	2022-02-05 20:22:06.000000000 +0200
> +++ linux-source-5.16.fix/arch/um/drivers/mconsole_kern.c	2022-02-16 23:35:39.562668086 +0200
> @@ -224,6 +224,7 @@
>   
>   void mconsole_stop(struct mc_request *req)
>   {
> +	int err;
>   	deactivate_fd(req->originating_fd, MCONSOLE_IRQ);
>   	os_set_fd_block(req->originating_fd, 1);
>   	mconsole_reply(req, "stopped", 0, 0);
> @@ -247,6 +248,11 @@
>   	}
>   	os_set_fd_block(req->originating_fd, 0);
>   	mconsole_reply(req, "", 0, 0);
> +	err=activate_fd(MCONSOLE_IRQ, req->originating_fd, IRQ_READ,
> +		(void*)(req->originating_fd), NULL);
> +	if (err)
> +	  mconsole_reply(req, "Failed to reactivate MCONSOLE_IRQ, \
> +			this will block the read for uml_mconsole", 1, 0);
>   }
>   
>   static DEFINE_SPINLOCK(mc_devices_lock);
> --- linux-source-5.16/arch/um/kernel/irq.c	2022-02-05 20:22:06.000000000 +0200
> +++ linux-source-5.16.fix/arch/um/kernel/irq.c	2022-02-16 23:39:15.650279367 +0200
> @@ -249,7 +249,7 @@
>   		free_irq_entry(entry, false);
>   }
>   
> -static int activate_fd(int irq, int fd, enum um_irq_type type, void *dev_id,
> +int activate_fd(int irq, int fd, enum um_irq_type type, void *dev_id,
>   		       void (*timetravel_handler)(int, int, void *,
>   						  struct time_travel_event *))
>   {
> @@ -304,6 +304,7 @@
>   out:
>   	return err;
>   }
> +EXPORT_SYMBOL(activate_fd);
>   
>   /*
>    * Remove the entry or entries for a specific FD, if you
> --- linux-source-5.16/arch/um/include/shared/irq_user.h	2022-02-05 20:22:06.000000000 +0200
> +++ linux-source-5.16.fix/arch/um/include/shared/irq_user.h	2022-02-16 23:39:09.642292312 +0200
> @@ -19,6 +19,7 @@
>   void sigio_run_timetravel_handlers(void);
>   extern void free_irq_by_fd(int fd);
>   extern void deactivate_fd(int fd, int irqnum);
> +extern int activate_fd(int irq, int fd, enum um_irq_type type, void *dev_id, void (*timetravel_handler)(int, int, void *, struct time_travel_event *));
>   extern int deactivate_all_fds(void);
>   extern int activate_ipi(int fd, int pid);
> 
> On Thu, 2022-02-17 at 01:06 +0200, Mihai Hanor wrote:
>> Package: user-mode-linux
>> Version: 5.16um1
>> Severity: normal
>> File: /usr/bin/linux.uml
>> X-Debbugs-Cc: quake2iasi at gmail.com
>>
>> Dear Maintainer,
>>
>> * What is the problem:
>> Issuing the commands stop followed by go, at the input of the
>> uml_mconsole
>> client, results in the client becoming blocked on read socket. This
>> is because
>> of logic in arch/um/drivers/mconsole_kern.c, where mconsole_stop()
>> doesn't
>> reactivate the MCONSOLE_IRQ before the function has exited.
>>
>> I've managed to find a fix which seems to be working, but I don't
>> know if it's
>> a proper fix. Please see the attached file.
>>
>>
>> -- System Information:
>> Debian Release: bookworm/sid
>>    APT prefers testing
>>    APT policy: (900, 'testing'), (500, 'unstable-debug'), (500,
>> 'testing-debug'), (1, 'unstable')
>> Architecture: amd64 (x86_64)
>> Foreign Architectures: i386
>>
>> Kernel: Linux 5.16.0-1-amd64 (SMP w/4 CPU threads; PREEMPT)
>> Kernel taint flags: TAINT_WARN
>> Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8),
>> LANGUAGE=en_US:en
>> Shell: /bin/sh linked to /bin/dash
>> Init: systemd (via /run/systemd/system)
>> LSM: AppArmor: enabled
>>
>> Versions of packages user-mode-linux depends on:
>> ii  libc6  2.33-5
>>
>> Versions of packages user-mode-linux recommends:
>> ii  uml-utilities  20070815.4-1
>>
>> Versions of packages user-mode-linux suggests:
>> ii  mate-terminal [x-terminal-emulator]  1.26.0-1
>> ii  pterm [x-terminal-emulator]          0.76-2
>> pn  rootstrap                            <none>
>> pn  slirp                                <none>
>> pn  user-mode-linux-doc                  <none>
>> pn  vde2                                 <none>
>>
>> -- no debconf information
> 
> 
> _______________________________________________
> linux-um mailing list
> linux-um at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-um
> 

-- 
Anton R. Ivanov
https://www.kot-begemot.co.uk/