From: Matthieu Herrb <matthieu@openbsd.org>
Subject: Re: xenocara/app/xidle corner locking and debug code fixes
To: Alexander Arkhipov <aa@manpager.org>
Cc: tech@openbsd.org
Date: Sat, 20 Jan 2024 10:48:36 +0100

On Sun, Jan 14, 2024 at 09:50:08AM +0300, Alexander Arkhipov wrote:
> Hello, tech@,
> 
> I noticed a bug in xidle: if you use the program with corner locking,
> and then put some window over that corner, then moving the pointer there
> doesn't actually activate xidle.
> 
> The bug is a little weird. There is code in xidle to handle
> VisibilityNotify, but it doesn't get executed because:
> 
> 1. VisibilityChangeMask is hidden behind an "#if 0"
> 2. The window is created as InputOnly instead of InputOutput

Hi,

This was explicitely disabled in 2005/10 by fgsch@, the original
author of xidle(1) with the comment :

  disable visibilitychangemask.. it has some hmm, side effects.

So I'd like to take a bit more time to understand what are those sides
effects or if they have been fixed by further changes since then.


> 
> The first diff below fixes both issues. I've been running this fixed
> version for several days now and noticed no problems, and there are no
> comments, so I suspect this was left in as a result of mistake.
> 
> Another issue I noticed while dealing with the first is that some of the
> debug printing doesn't work because at one point std{in,out,err} get
> redirected to /dev/null even if DEBUG is defined. The second diff fixes
> that. I've also had an idea of instead turning everything hidden behind
> "#if*def DEBUG"s into a run-time option (perhaps -v or -debug), but I'd
> like to see some feedback before I do that.

I prefer to leave the debug code as a compile-time option. It's not
something regular users (who do not plan to change the code) need
everyday. Less code -> less attack surface.

> 
> Lastly, I'm not sure if the two diffs will apply on top of each other,
> but both are trivial enough to be typed in by hand.
> 
> These are my very first diffs to tech@, so I'd appreciate some feedback.
> Thank you.
> 
> Index: xidle.c
> ===================================================================
> RCS file: /var/cvs/xenocara/app/xidle/xidle.c,v
> diff -u -p -r1.10 xidle.c
> --- xidle.c	25 Oct 2021 09:30:33 -0000	1.10
> +++ xidle.c	14 Jan 2024 06:15:14 -0000
> @@ -129,15 +129,12 @@ init_x(struct xinfo *xi, int position, i
>  
>  	attr.override_redirect = True;
>  	xi->win = XCreateWindow(dpy, DefaultRootWindow(dpy),
> -	    xi->coord_x, xi->coord_y, area, area, 0, 0, InputOnly,
> +	    xi->coord_x, xi->coord_y, area, area, 0, 0, InputOutput,
>  	    CopyFromParent, CWOverrideRedirect, &attr);
>  
>  	if (position != none) {
>  		XSelectInput(dpy, xi->win, EnterWindowMask|StructureNotifyMask
> -#if 0
> -			       |VisibilityChangeMask
> -#endif
> -		);
> +		    |VisibilityChangeMask);
>  		XMapWindow(dpy, xi->win);
>  	}
>  
> 
> And the second diff:
> 
> 
> Index: xidle.c
> ===================================================================
> RCS file: /var/cvs/xenocara/app/xidle/xidle.c,v
> diff -u -p -r1.10 xidle.c
> --- xidle.c	25 Oct 2021 09:30:33 -0000	1.10
> +++ xidle.c	14 Jan 2024 06:16:37 -0000
> @@ -327,7 +327,9 @@ main(int argc, char **argv)
>  	char *args[10];
>  	int area = 2, delay = 2, timeout = 0;
>  	int position = north|west;
> +#ifndef DEBUG
>  	int fd;
> +#endif
>  	u_long last_serial = 0;
>  
>  	parse_opts(argc, argv, &x.dpy, &area, &delay, &timeout,
> @@ -345,6 +347,7 @@ main(int argc, char **argv)
>  	signal(SIGTERM, handler);
>  	signal(SIGUSR1, handler);
>  
> +#ifndef DEBUG
>  	fd = open(_PATH_DEVNULL, O_RDWR);
>  	if (fd < 0)
>  		err(1, _PATH_DEVNULL);
> @@ -353,6 +356,7 @@ main(int argc, char **argv)
>  	dup2(fd, STDERR_FILENO);
>  	if (fd > 2)
>  		close(fd);
> +#endif
>  
>  	if (pledge("stdio proc exec", NULL) == -1)
>  		err(1, "pledge");
> 
> -- 
> Alexander
>