The status bar in drawbar() calculates the text width as TEXTW(stext)
- lrpad + 2. However, the click detection in buttonpress() used
TEXTW(stext) without adjusting for that padding.
This created an "extra" clickable area of some pixels to the left of
the status text that would incorrectly trigger ClkStatusText actions
instead of ClkWinTitle.
Steps to reproduce:
1. Set a status text: xsetroot -name "HELLO"
2. Move the mouse to the empty space with some pixels close to the
left of the word "HELLO" but in the title area.
3. Middle-click (or any binding for ClkStatusText).
4. You can see that the status bar action is triggered (default a
terminal spawns), even though you clicked in the window title area.
This fix ensures that the clickable area matches the visual text.
When a fullscreen window is moved to another monitor (e.g. via
tagmon), its geometry does not always match the new monitor's
dimensions.
Steps to reproduce:
1. Start dwm with two monitors (A and B).
2. Open a window on Monitor A.
3. Make the window fullscreen (e.g. Firefox with F11).
4. Move the window to Monitor B using the tagmon shortcut (Mod+Shift+>).
5. Go to the other monitor (B), observe that the window is still
visible on Monitor A and its contents, even though the window's title
is seen on Monitor B bar.
6. Go to the monitor A where the window is still in fullscreen, remove
the fullscreen and the window automatically will go to monitor B.
This fix ensures that fullscreen windows are correctly resized to the
new monitor's geometry during the move.
commit 244fa852 (and a9aa0d8) tried to fix overflow by checking
the number of items returned. however this is not sufficient
since the format may be lower than 32 bits.
to reproduce the crash, i used the reproducer given in commit
244fa85 but changed the XChangeProperty line to the following to
set the property to a 1 element 16 bit item:
short si = 1;
XChangeProperty(d, w, net_wm_state, XA_ATOM, 16,
PropModeReplace, (unsigned char *)&si, 1);
this client reliably crashes dwm under ASAN since dwm is trying
to read a 32 bit value from a 16 bit one. fix it by checking for
format == 32 as well.
also change the access type from Atom to long, on my machine
Atom is typedef-ed to long already but that may not be true
everywere. the XGetWindowProperty manpage says format == 32 is
returned as `long` so use `long` directly.
(N.B: it also might be worth checking if the returned type is
XA_ATOM as well, but i wasn't able to cause any crashes by
setting different types so i'm leaving it out for now.)
WM_STATE is defined to be format == 32 which xlib returns as
`long` and so accessing it as `unsigned char` is incorrect.
and also &p is already an `unsigned char **` and so the cast was
completely redundant.
given the redundant cast, i assume `p` was `long *` at some time
but was changed to `unsigned char *` later, but the pointer
access (and the cast) wasn't updated.
also add a `format == 32` check as safety measure before
accessing, just in case.
currently clients that set the input field of WM_HINTS to true
(c->neverfocus) will never be updated as _NET_ACTIVE_WINDOW even
when they are focused. according to the ICCCM [0] the input
field of WM_HINTS tells the WM to either use or not use
XSetInputFocus(), it shouldn't have any relation to
_NET_ACTIVE_WINDOW. EWMH spec [1] also does not mention any
relationship between the two.
this issue was noticed when launching games via steam/proton and
noticing that _NET_ACTIVE_WINDOW was always wrong/stale (i.e not
updated to the game window).
for reference I've looked at bspwm [2] and it also seems to set
_NET_ACTIVE_WINDOW regardless of whether the client has WM_HINTS
input true or not.
[0]: https://x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#input_focus
[1]: https://specifications.freedesktop.org/wm/1.5/ar01s03.html#id-1.4.10
[2]: c5cf7d3943/src/tree.c (L659-L662)
Commit 244fa852fe ("dwm: Fix heap buffer overflow in getatomprop")
introduced a check for dl > 0 before dereferencing the property pointer.
However, I missed that the variable dl is passed to XGetWindowProperty
for both nitems_return and bytes_after_return parameters:
XGetWindowProperty(..., &dl, &dl, &p)
The final value in dl is bytes_after_return, not nitems_return. For a
successfully read property, bytes_after is typically 0 (indicating all
data was retrieved), so the check `dl > 0` is always false and dwm never
reads any atom properties. So this is safe, but not very helpful :-)
dl is probably just a dummy variable anyway, so fix by using a separate
variable for nitems, and check nitems > 0 as originally intended.
When getatomprop() is called, it invokes XGetWindowProperty() to
retrieve an Atom. If the property exists but has zero elements (length
0), Xlib returns Success and sets p to a valid, non-NULL memory address
containing a single null byte.
However, dl (that is, the number of items) is 0. dwm blindly casts p to
Atom* and dereferences it. While Xlib guarantees that p is safe to read
as a string (that is, it is null-terminated), it does _not_ guarantee it
is safe to read as an Atom (an unsigned long).
The Atom type is a typedef for unsigned long. Reading an Atom (which
thus will either likely be 4 or 8 bytes) from a 1-byte allocated buffer
results in a heap buffer overflow. Since property content is user
controlled, this allows any client to trigger an out of bounds read
simply by setting a property with format 32 and length 0.
An example client which reliably crashes dwm under ASAN:
#include <X11/Xlib.h>
#include <X11/Xatom.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
int main(void) {
Display *d;
Window root, w;
Atom net_wm_state;
d = XOpenDisplay(NULL);
if (!d) return 1;
root = DefaultRootWindow(d);
w = XCreateSimpleWindow(d, root, 10, 10, 200, 200, 1, 0, 0);
net_wm_state = XInternAtom(d, "_NET_WM_STATE", False);
if (net_wm_state == None) return 1;
XChangeProperty(d, w, net_wm_state, XA_ATOM, 32,
PropModeReplace, NULL, 0);
XMapWindow(d, w);
XSync(d, False);
sleep(1);
XCloseDisplay(d);
return 0;
}
In order to avoid this, check that the number of items returned is
greater than zero before dereferencing the pointer.
- drw: minor improvement to the nomatches cache
- overhaul utf8decoding and render invalid utf8 sequences as U+FFFD.
Thanks NRK for these improvements!
The Makefile used to suppress output (by using @), so this target made sense at
the time.
But the Makefile should be simple and make debugging with less abstractions or
fancy printing. The Makefile was made verbose and doesn't hide the build
output, so remove this target.
Prompted by a question on the mailing list about the options target.
From sigaction(2):
A child created via fork(2) inherits a copy of its parent's signal dispositions.
During an execve(2), the dispositions of handled signals are reset to the default;
the dispositions of ignored signals are left unchanged.
This refused to start directly some programs from configuring in config.h:
static Key keys[] = {
MODKEY, XK_o, spawn, {.v = cmd } },
};
Some reported programs that didn't start were: mpv, anki, dmenu_extended.
Reported by pfx.
Initial patch suggestion by Storkman.
SA_NOCLDWAIT is marked as XSI in the posix spec [0] and FreeBSD and NetBSD
seems to more be strict about the feature test macro [1].
so update the macro to use _XOPEN_SOURCE=700L instead, which is equivalent to
_POSIX_C_SOURCE=200809L except that it also unlocks the X/Open System
Interfaces.
[0]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html#tag_13_42
[1]: https://lists.suckless.org/dev/2302/35111.html
Tested on:
* NetBSD 9.3 (fixed).
* FreeBSD 13 (fixed).
* Void Linux musl.
* Void Linux glibc.
* OpenBSD 7.2 (stable).
* Slackware 11.
Reported-by: beastie <pufferfish@riseup.net>
signal() semantics are pretty unclearly specified. For example, depending on OS
kernel and libc, the handler may be returned to SIG_DFL (hence the inner call
to read the signal handler). Moving to sigaction() means the behaviour is
consistently defined.
Using SA_NOCLDWAIT also allows us to avoid calling the non-reentrant function
die() in the handler.
Some addditional notes for archival purposes:
* NRK pointed out errno of waitpid could also theoretically get clobbered.
* The original patch was iterated on and modified by NRK and Hiltjo:
* SIG_DFL was changed to SIG_IGN, this is required, atleast on older systems
such as tested on Slackware 11.
* signals are not blocked using sigprocmask, because in theory it would
briefly for example also ignore a SIGTERM signal. It is OK if waitpid() is (in
theory interrupted).
POSIX reference:
"Consequences of Process Termination":
https://pubs.opengroup.org/onlinepubs/9699919799/functions/_Exit.html#tag_16_01_03_01
It's not uncommon for one keysym to map to multiple keycodes. For
example, the "play" button on my keyboard sends keycode 172, but my
bluetooth headphones send keycode 208, both of which map back to
XF86AudioPlay:
% xmodmap -pke | grep XF86AudioPlay
keycode 172 = XF86AudioPlay XF86AudioPause XF86AudioPlay XF86AudioPause
keycode 208 = XF86AudioPlay NoSymbol XF86AudioPlay
keycode 215 = XF86AudioPlay NoSymbol XF86AudioPlay
This is a problem because the current code only grabs a single one of
these keycodes, which means that events for any other keycode also
mapping to the bound keysym will not be handled by dwm. In my case, this
means that binding XF86AudioPlay does the right thing and correctly
handles my keyboard's keys, but does nothing on my headphones. I'm not
the only person affected by this, there are other reports[0].
In order to fix this, we look at the mappings between keycodes and
keysyms at grabkeys() time and pick out all matching keycodes rather
than just the first one. The keypress() side of this doesn't need any
changes because the keycode gets converted back to a canonical keysym
before any action is taken.
0: https://github.com/cdown/dwm/issues/11
This reverts commit c2b748e793.
Revert back this change. It seems to not be an edge-case anymore since
multiple users have asked about this new behaviour now.
Reasoning: Since 2011 dmenu has been capable of working out which
monitor currently has focus in a Xinerama setup, making the use
of the -m flag more or less redundant.
This is easily demonstrated by using dmenu in any other window
manager.
There used to be a nodmenu patch that provided these changes:
https://git.suckless.org/sites/commit/ed68e3629de4ef2ca2d3f8893a79fb570b4c0cbc.html
but this was removed on the basis that it was very easy to work
out and apply manually if needed.
The proposal here is to remove this dependency from dwm. The
mechanism of the dmenumon variable could be provided via a patch
if need be.
The edge case scenario that dmenu does not handle on its own, and
the effect of removing this mechanism, is that if the user trigger
focusmon via keybindings to change focus to another monitor that
has no clients, then dmenu will open on the monitor containing the
window with input focus (or the monitor with the mouse cursor if
no windows have input focus).
If this edge case is important to cover then this can be addressed
by setting input focus to selmon->barwin in the focus function if
there is no client to give focus to (rather than giving focus back
to the root window).
