udev-builtin-input_id: use heuristics to detect joysticks

Several keyboard devices are erroneously tagged with ID_INPUT_JOYSTICK
because of random buttons they set. For example, the LiteOn Lenovo
Calliope USB Keyboard sets BTN_TRIGGER, BTN_TOP2, BTN_PINKIE and
BTN_BASE, see libinput issue 745 for details.

ID_INPUT_JOYSTICK triggers the uaccess rules, making those keyboards
easily accessible. That's not a problem in the LiteOn example since that
event node doesn't contain the normal keys and eavesdropping on volume
keys is probably not very interesting.

Improve the joystick detection by adding heuristics similar to what
libinput 1.20 uses: check for some specific set of keys that are common
on keyboards but very unlikely on joysticks. If enough of those are
present (or the device has less than 2 axes or joysticks), don't tag it
as joystick.

libinput also checks for > 10 keyboard keys, but this is not done here
to be more conservative.

man/standard-specifiers.xml

@@ -4,6 +4,9 @@
 <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
 
 <tbody>
+  <!-- note: units also use the following deprecated specifiers that are not documented:
+       %c, %r, %R. Do not reuse. -->
+
   <row id='a'>
     <entry><literal>%a</literal></entry>
     <entry>Architecture</entry>

src/core/unit-printf.c

@@ -195,8 +195,8 @@ int unit_name_printf(const Unit *u, const char* format, char **ret) {
 }
 
 int unit_full_printf_full(const Unit *u, const char *format, size_t max_length, char **ret) {
-        /* This is similar to unit_name_printf() but also supports unescaping. Also, adds a couple of additional codes
-         * (which are likely not suitable for unescaped inclusion in unit names):
+        /* This is similar to unit_name_printf() but also supports unescaping. Also, adds a couple of
+         * additional codes (which are likely not suitable for unescaped inclusion in unit names):
          *
          * %f: the unescaped instance if set, otherwise the id unescaped as path
          *
@@ -214,9 +214,9 @@ int unit_full_printf_full(const Unit *u, const char *format, size_t max_length,
          * %h: the homedir of the running user
          * %s: the shell of the running user
          *
-         * NOTICE: When you add new entries here, please be careful: specifiers which depend on settings of the unit
-         * file itself are broken by design, as they would resolve differently depending on whether they are used
-         * before or after the relevant configuration setting. Hence: don't add them.
+         * NOTICE: When you add new entries here, please be careful: specifiers which depend on settings of
+         * the unit file itself are broken by design, as they would resolve differently depending on whether
+         * they are used before or after the relevant configuration setting. Hence: don't add them.
          */
 
         assert(u);
@@ -237,9 +237,9 @@ int unit_full_printf_full(const Unit *u, const char *format, size_t max_length,
                 { 'y', specifier_real_path,                u->fragment_path },
                 { 'Y', specifier_real_directory,           u->fragment_path },
 
-                { 'c', specifier_cgroup,                   NULL },
-                { 'r', specifier_cgroup_slice,             NULL },
-                { 'R', specifier_cgroup_root,              NULL },
+                { 'c', specifier_cgroup,                   NULL },  /* deprecated, see 1b89b0c499cd4bf0ff389caab4ecaae6e75f9d4e */
+                { 'r', specifier_cgroup_slice,             NULL },  /* deprecated, see 1b89b0c499cd4bf0ff389caab4ecaae6e75f9d4e */
+                { 'R', specifier_cgroup_root,              NULL },  /* deprecated, see 1b89b0c499cd4bf0ff389caab4ecaae6e75f9d4e */
 
                 { 'C', specifier_special_directory,        UINT_TO_PTR(EXEC_DIRECTORY_CACHE) },
                 { 'd', specifier_credentials_dir,          NULL },

src/libsystemd/sd-device/device-enumerator.c

@@ -638,7 +638,12 @@ static bool match_subsystem(sd_device_enumerator *enumerator, const char *subsys
         return false;
 }
 
-static int enumerator_scan_dir(sd_device_enumerator *enumerator, const char *basedir, const char *subdir, const char *subsystem) {
+static int enumerator_scan_dir(
+                sd_device_enumerator *enumerator,
+                const char *basedir,
+                const char *subdir,
+                const char *subsystem) {
+
         _cleanup_closedir_ DIR *dir = NULL;
         char *path;
         int r = 0;
@@ -997,7 +1002,7 @@ _public_ sd_device *sd_device_enumerator_get_subsystem_next(sd_device_enumerator
 }
 
 int device_enumerator_scan_devices_and_subsystems(sd_device_enumerator *enumerator) {
-        int r = 0, k;
+        int r;
 
         assert(enumerator);
 
@@ -1007,22 +1012,14 @@ int device_enumerator_scan_devices_and_subsystems(sd_device_enumerator *enumerat
 
         device_enumerator_unref_devices(enumerator);
 
-        if (!set_isempty(enumerator->match_tag)) {
-                k = enumerator_scan_devices_tags(enumerator);
-                if (k < 0)
-                        r = k;
-        } else if (enumerator->match_parent) {
-                k = enumerator_scan_devices_children(enumerator);
-                if (k < 0)
-                        r = k;
-        } else {
-                k = enumerator_scan_dir(enumerator, "class", NULL, NULL);
-                if (k < 0)
-                        r = log_debug_errno(k, "sd-device-enumerator: Failed to scan /sys/class: %m");
+        if (!set_isempty(enumerator->match_tag))
+                r = enumerator_scan_devices_tags(enumerator);
+        else if (enumerator->match_parent)
+                r = enumerator_scan_devices_children(enumerator);
+        else {
+                int k;
 
-                k = enumerator_scan_dir(enumerator, "bus", "devices", NULL);
-                if (k < 0)
-                        r = log_debug_errno(k, "sd-device-enumerator: Failed to scan /sys/bus: %m");
+                r = enumerator_scan_devices_all(enumerator);
 
                 if (match_subsystem(enumerator, "module")) {
                         k = enumerator_scan_dir_and_add_devices(enumerator, "module", NULL, NULL);

src/libsystemd/sd-device/sd-device.c

@@ -207,10 +207,11 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) {
                 syspath = strdup(_syspath);
                 if (!syspath)
                         return log_oom_debug();
+
+                path_simplify(syspath);
         }
 
-        devpath = syspath + STRLEN("/sys");
-
+        assert_se(devpath = startswith(syspath, "/sys"));
         if (devpath[0] != '/')
                 return log_debug_errno(SYNTHETIC_ERRNO(ENODEV), "sd-device: \"/sys\" alone is not a valid device path.");
 
@@ -584,11 +585,15 @@ int device_set_devnum(sd_device *device, const char *major, const char *minor) {
                 return r;
         if (maj == 0)
                 return 0;
+        if (!DEVICE_MAJOR_VALID(maj))
+                return -EINVAL;
 
         if (minor) {
                 r = safe_atou(minor, &min);
                 if (r < 0)
                         return r;
+                if (!DEVICE_MINOR_VALID(min))
+                        return -EINVAL;
         }
 
         r = device_add_property_internal(device, "MAJOR", major);

src/resolve/resolved-conf.c

@@ -225,7 +225,7 @@ int config_parse_dnssd_service_name(
                 { 'a', specifier_architecture,    NULL },
                 { 'b', specifier_boot_id,         NULL },
                 { 'B', specifier_os_build_id,     NULL },
-                { 'H', specifier_host_name,       NULL }, /* We will use specifier_dnssd_host_name(). */
+                { 'H', specifier_hostname,        NULL }, /* We will use specifier_dnssd_hostname(). */
                 { 'm', specifier_machine_id,      NULL },
                 { 'o', specifier_os_id,           NULL },
                 { 'v', specifier_kernel_release,  NULL },

src/resolve/resolved-dnssd.c

@@ -135,7 +135,7 @@ static int dnssd_service_load(Manager *manager, const char *filename) {
         return 0;
 }
 
-static int specifier_dnssd_host_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
+static int specifier_dnssd_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
         DnssdService *s  = (DnssdService *) userdata;
         char *n;
 
@@ -156,7 +156,7 @@ int dnssd_render_instance_name(DnssdService *s, char **ret_name) {
                 { 'a', specifier_architecture,   NULL },
                 { 'b', specifier_boot_id,        NULL },
                 { 'B', specifier_os_build_id,    NULL },
-                { 'H', specifier_dnssd_host_name, NULL },
+                { 'H', specifier_dnssd_hostname, NULL },
                 { 'm', specifier_machine_id,     NULL },
                 { 'o', specifier_os_id,          NULL },
                 { 'v', specifier_kernel_release, NULL },

src/shared/specifier.c

@@ -199,7 +199,7 @@ int specifier_boot_id(char specifier, const void *data, const char *root, const
         return 0;
 }
 
-int specifier_host_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
+int specifier_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
         char *n;
 
         assert(ret);
@@ -212,7 +212,7 @@ int specifier_host_name(char specifier, const void *data, const char *root, cons
         return 0;
 }
 
-int specifier_short_host_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
+int specifier_short_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
         char *n;
 
         assert(ret);
@@ -225,7 +225,7 @@ int specifier_short_host_name(char specifier, const void *data, const char *root
         return 0;
 }
 
-int specifier_pretty_host_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
+int specifier_pretty_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret) {
         char *n = NULL;
 
         assert(ret);
@@ -276,12 +276,18 @@ int specifier_architecture(char specifier, const void *data, const char *root, c
  * installation. */
 
 static int parse_os_release_specifier(const char *root, const char *id, char **ret) {
+        char *v = NULL;
         int r;
 
         assert(ret);
 
+        r = parse_os_release(root, id, &v);
+        if (r >= 0)
+                /* parse_os_release() calls parse_env_file() which only sets the return value for
+                 * entries found. Let's make sure we set the return value in all cases. */
+                *ret = v;
+
         /* Translate error for missing os-release file to EUNATCH. */
-        r = parse_os_release(root, id, ret);
         return r == -ENOENT ? -EUNATCH : r;
 }
 

src/shared/specifier.h

@@ -19,9 +19,9 @@ int specifier_real_directory(char specifier, const void *data, const char *root,
 
 int specifier_machine_id(char specifier, const void *data, const char *root, const void *userdata, char **ret);
 int specifier_boot_id(char specifier, const void *data, const char *root, const void *userdata, char **ret);
-int specifier_host_name(char specifier, const void *data, const char *root, const void *userdata, char **ret);
-int specifier_short_host_name(char specifier, const void *data, const char *root, const void *userdata, char **ret);
-int specifier_pretty_host_name(char specifier, const void *data, const char *root, const void *userdata, char **ret);
+int specifier_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret);
+int specifier_short_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret);
+int specifier_pretty_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret);
 int specifier_kernel_release(char specifier, const void *data, const char *root, const void *userdata, char **ret);
 int specifier_architecture(char specifier, const void *data, const char *root, const void *userdata, char **ret);
 int specifier_os_id(char specifier, const void *data, const char *root, const void *userdata, char **ret);
@@ -75,9 +75,9 @@ int specifier_var_tmp_dir(char specifier, const void *data, const char *root, co
         { 'A', specifier_os_image_version, NULL }, \
         { 'b', specifier_boot_id,          NULL }, \
         { 'B', specifier_os_build_id,      NULL }, \
-        { 'H', specifier_host_name,       NULL }, \
-        { 'l', specifier_short_host_name, NULL }, \
-        { 'q', specifier_pretty_host_name,NULL }, \
+        { 'H', specifier_hostname,         NULL }, \
+        { 'l', specifier_short_hostname,   NULL }, \
+        { 'q', specifier_pretty_hostname,  NULL }, \
         { 'm', specifier_machine_id,       NULL }, \
         { 'M', specifier_os_image_id,      NULL }, \
         { 'o', specifier_os_id,            NULL }, \

src/test/test-unit-name.c

@@ -16,6 +16,7 @@
 #include "specifier.h"
 #include "string-util.h"
 #include "tests.h"
+#include "tmpfile-util.h"
 #include "unit-def.h"
 #include "unit-name.h"
 #include "unit-printf.h"
@@ -213,42 +214,75 @@ TEST(unit_name_mangle) {
 }
 
 TEST_RET(unit_printf, .sd_booted = true) {
-        _cleanup_free_ char *mid = NULL, *bid = NULL, *host = NULL, *gid = NULL, *group = NULL, *uid = NULL, *user = NULL, *shell = NULL, *home = NULL;
+        _cleanup_free_ char
+                *architecture, *os_image_version, *boot_id, *os_build_id,
+                *hostname, *short_hostname, *pretty_hostname,
+                *machine_id, *os_image_id, *os_id, *os_version_id, *os_variant_id,
+                *user, *group, *uid, *gid, *home, *shell,
+                *tmp_dir, *var_tmp_dir;
         _cleanup_(manager_freep) Manager *m = NULL;
         Unit *u;
         int r;
 
-        assert_se(specifier_machine_id('m', NULL, NULL, NULL, &mid) >= 0 && mid);
-        assert_se(specifier_boot_id('b', NULL, NULL, NULL, &bid) >= 0 && bid);
-        assert_se(host = gethostname_malloc());
+        _cleanup_(unlink_tempfilep) char filename[] = "/tmp/test-unit_printf.XXXXXX";
+        assert_se(mkostemp_safe(filename) >= 0);
+
+        /* Using the specifier functions is admittedly a bit circular, but we don't want to reimplement the
+         * logic a second time. We're at least testing that the hookup works. */
+        assert_se(specifier_architecture('a', NULL, NULL, NULL, &architecture) >= 0);
+        assert_se(architecture);
+        assert_se(specifier_os_image_version('A', NULL, NULL, NULL, &os_image_version) >= 0);
+        assert_se(specifier_boot_id('b', NULL, NULL, NULL, &boot_id) >= 0);
+        assert_se(boot_id);
+        assert_se(specifier_os_build_id('B', NULL, NULL, NULL, &os_build_id) >= 0);
+        assert_se(hostname = gethostname_malloc());
+        assert_se(specifier_short_hostname('l', NULL, NULL, NULL, &short_hostname) == 0);
+        assert_se(short_hostname);
+        assert_se(specifier_pretty_hostname('q', NULL, NULL, NULL, &pretty_hostname) == 0);
+        assert_se(pretty_hostname);
+        assert_se(specifier_machine_id('m', NULL, NULL, NULL, &machine_id) >= 0);
+        assert_se(machine_id);
+        assert_se(specifier_os_image_id('M', NULL, NULL, NULL, &os_image_id) >= 0);
+        assert_se(specifier_os_id('o', NULL, NULL, NULL, &os_id) >= 0);
+        assert_se(specifier_os_version_id('w', NULL, NULL, NULL, &os_version_id) >= 0);
+        assert_se(specifier_os_variant_id('W', NULL, NULL, NULL, &os_variant_id) >= 0);
         assert_se(user = uid_to_name(getuid()));
         assert_se(group = gid_to_name(getgid()));
         assert_se(asprintf(&uid, UID_FMT, getuid()));
         assert_se(asprintf(&gid, UID_FMT, getgid()));
         assert_se(get_home_dir(&home) >= 0);
         assert_se(get_shell(&shell) >= 0);
+        assert_se(specifier_tmp_dir('T', NULL, NULL, NULL, &tmp_dir) >= 0);
+        assert_se(tmp_dir);
+        assert_se(specifier_var_tmp_dir('V', NULL, NULL, NULL, &var_tmp_dir) >= 0);
+        assert_se(var_tmp_dir);
 
         r = manager_new(LOOKUP_SCOPE_USER, MANAGER_TEST_RUN_MINIMAL, &m);
         if (manager_errno_skip_test(r))
                 return log_tests_skipped_errno(r, "manager_new");
         assert_se(r == 0);
 
-#define expect(unit, pattern, expected)                                 \
+        assert_se(free_and_strdup(&m->cgroup_root, "/cgroup-root") == 1);
+
+#define expect(unit, pattern, _expected)                                \
         {                                                               \
-                char *e;                                                \
                 _cleanup_free_ char *t = NULL;                          \
                 assert_se(unit_full_printf(unit, pattern, &t) >= 0);    \
-                printf("result: %s\nexpect: %s\n", t, expected);        \
-                if ((e = endswith(expected, "*")))                      \
-                        assert_se(strncmp(t, e, e-expected));              \
-                else                                                    \
-                        assert_se(streq(t, expected));                     \
+                const char *expected = strempty(_expected);             \
+                printf("%s: result: %s\n    expect: %s\n", pattern, t, expected); \
+                assert_se(fnmatch(expected, t, FNM_NOESCAPE) == 0);     \
         }
 
         assert_se(u = unit_new(m, sizeof(Service)));
         assert_se(unit_add_name(u, "blah.service") == 0);
         assert_se(unit_add_name(u, "blah.service") == 0);
 
+        /* We need *a* file that exists, but it doesn't even need to have the right suffix. */
+        assert_se(free_and_strdup(&u->fragment_path, filename) == 1);
+
+        /* This sets the slice to /app.slice. */
+        assert_se(unit_set_default_slice(u) == 1);
+
         /* general tests */
         expect(u, "%%", "%");
         expect(u, "%%s", "%s");
@@ -256,62 +290,103 @@ TEST_RET(unit_printf, .sd_booted = true) {
         expect(u, "%", "%");
 
         /* normal unit */
-        expect(u, "%n", "blah.service");
-        expect(u, "%f", "/blah");
-        expect(u, "%N", "blah");
-        expect(u, "%p", "blah");
-        expect(u, "%P", "blah");
-        expect(u, "%i", "");
-        expect(u, "%I", "");
-        expect(u, "%j", "blah");
-        expect(u, "%J", "blah");
+        expect(u, "%a", architecture);
+        expect(u, "%A", os_image_version);
+        expect(u, "%b", boot_id);
+        expect(u, "%B", os_build_id);
+        expect(u, "%H", hostname);
+        expect(u, "%l", short_hostname);
+        expect(u, "%q", pretty_hostname);
+        expect(u, "%m", machine_id);
+        expect(u, "%M", os_image_id);
+        expect(u, "%o", os_id);
+        expect(u, "%w", os_version_id);
+        expect(u, "%W", os_variant_id);
         expect(u, "%g", group);
         expect(u, "%G", gid);
         expect(u, "%u", user);
         expect(u, "%U", uid);
+        expect(u, "%T", tmp_dir);
+        expect(u, "%V", var_tmp_dir);
+
+        expect(u, "%i", "");
+        expect(u, "%I", "");
+        expect(u, "%j", "blah");
+        expect(u, "%J", "blah");
+        expect(u, "%n", "blah.service");
+        expect(u, "%N", "blah");
+        expect(u, "%p", "blah");
+        expect(u, "%P", "blah");
+        expect(u, "%f", "/blah");
+        expect(u, "%y", filename);
+        expect(u, "%Y", "/tmp");
+        expect(u, "%C", m->prefix[EXEC_DIRECTORY_CACHE]);
+        expect(u, "%d", "*/credentials/blah.service");
+        expect(u, "%E", m->prefix[EXEC_DIRECTORY_CONFIGURATION]);
+        expect(u, "%L", m->prefix[EXEC_DIRECTORY_LOGS]);
+        expect(u, "%S", m->prefix[EXEC_DIRECTORY_STATE]);
+        expect(u, "%t", m->prefix[EXEC_DIRECTORY_RUNTIME]);
         expect(u, "%h", home);
-        expect(u, "%m", mid);
-        expect(u, "%b", bid);
-        expect(u, "%H", host);
-        expect(u, "%t", "/run/user/*");
+        expect(u, "%s", shell);
+
+        /* deprecated */
+        expect(u, "%c", "/cgroup-root/app.slice/blah.service");
+        expect(u, "%r", "/cgroup-root/app.slice");
+        expect(u, "%R", "/cgroup-root");
 
         /* templated */
         assert_se(u = unit_new(m, sizeof(Service)));
         assert_se(unit_add_name(u, "blah@foo-foo.service") == 0);
         assert_se(unit_add_name(u, "blah@foo-foo.service") == 0);
 
-        expect(u, "%n", "blah@foo-foo.service");
-        expect(u, "%N", "blah@foo-foo");
-        expect(u, "%f", "/foo/foo");
-        expect(u, "%p", "blah");
-        expect(u, "%P", "blah");
+        assert_se(free_and_strdup(&u->fragment_path, filename) == 1);
+
+        /* This sets the slice to /app.slice/app-blah.slice. */
+        assert_se(unit_set_default_slice(u) == 1);
+
         expect(u, "%i", "foo-foo");
         expect(u, "%I", "foo/foo");
         expect(u, "%j", "blah");
         expect(u, "%J", "blah");
-        expect(u, "%g", group);
-        expect(u, "%G", gid);
-        expect(u, "%u", user);
-        expect(u, "%U", uid);
+        expect(u, "%n", "blah@foo-foo.service");
+        expect(u, "%N", "blah@foo-foo");
+        expect(u, "%p", "blah");
+        expect(u, "%P", "blah");
+        expect(u, "%f", "/foo/foo");
+        expect(u, "%y", filename);
+        expect(u, "%Y", "/tmp");
+        expect(u, "%C", m->prefix[EXEC_DIRECTORY_CACHE]);
+        expect(u, "%d", "*/credentials/blah@foo-foo.service");
+        expect(u, "%E", m->prefix[EXEC_DIRECTORY_CONFIGURATION]);
+        expect(u, "%L", m->prefix[EXEC_DIRECTORY_LOGS]);
+        expect(u, "%S", m->prefix[EXEC_DIRECTORY_STATE]);
+        expect(u, "%t", m->prefix[EXEC_DIRECTORY_RUNTIME]);
         expect(u, "%h", home);
-        expect(u, "%m", mid);
-        expect(u, "%b", bid);
-        expect(u, "%H", host);
-        expect(u, "%t", "/run/user/*");
+        expect(u, "%s", shell);
+
+        /* deprecated */
+        expect(u, "%c", "/cgroup-root/app.slice/app-blah.slice/blah@foo-foo.service");
+        expect(u, "%r", "/cgroup-root/app.slice/app-blah.slice");
+        expect(u, "%R", "/cgroup-root");
 
         /* templated with components */
         assert_se(u = unit_new(m, sizeof(Slice)));
         assert_se(unit_add_name(u, "blah-blah\\x2d.slice") == 0);
 
-        expect(u, "%n", "blah-blah\\x2d.slice");
-        expect(u, "%N", "blah-blah\\x2d");
-        expect(u, "%f", "/blah/blah-");
-        expect(u, "%p", "blah-blah\\x2d");
-        expect(u, "%P", "blah/blah-");
         expect(u, "%i", "");
         expect(u, "%I", "");
         expect(u, "%j", "blah\\x2d");
         expect(u, "%J", "blah-");
+        expect(u, "%n", "blah-blah\\x2d.slice");
+        expect(u, "%N", "blah-blah\\x2d");
+        expect(u, "%p", "blah-blah\\x2d");
+        expect(u, "%P", "blah/blah-");
+        expect(u, "%f", "/blah/blah-");
+
+        /* deprecated */
+        expect(u, "%c", "/cgroup-root/blah-blah\\x2d.slice");
+        expect(u, "%r", "/cgroup-root");
+        expect(u, "%R", "/cgroup-root");
 
 #undef expect
 

src/tmpfiles/tmpfiles.c

@@ -2923,8 +2923,8 @@ static int parse_line(
                 { 'a', specifier_architecture,    NULL },
                 { 'b', specifier_boot_id,         NULL },
                 { 'B', specifier_os_build_id,     NULL },
-                { 'H', specifier_host_name,       NULL },
-                { 'l', specifier_short_host_name, NULL },
+                { 'H', specifier_hostname,        NULL },
+                { 'l', specifier_short_hostname,  NULL },
                 { 'm', specifier_machine_id_safe, NULL },
                 { 'o', specifier_os_id,           NULL },
                 { 'v', specifier_kernel_release,  NULL },

src/udev/udev-builtin-input_id.c

@@ -156,7 +156,8 @@ static bool test_pointers(sd_device *dev,
         bool has_abs_coordinates = false;
         bool has_rel_coordinates = false;
         bool has_mt_coordinates = false;
-        bool has_joystick_axes_or_buttons = false;
+        size_t num_joystick_axes = 0;
+        size_t num_joystick_buttons = 0;
         bool has_pad_buttons = false;
         bool is_direct = false;
         bool has_touch = false;
@@ -214,15 +215,19 @@ static bool test_pointers(sd_device *dev,
          * Catz Mad Catz M.M.O.TE). Skip those.
          */
         if (!test_bit(BTN_JOYSTICK - 1, bitmask_key)) {
-                for (int button = BTN_JOYSTICK; button < BTN_DIGI && !has_joystick_axes_or_buttons; button++)
-                        has_joystick_axes_or_buttons = test_bit(button, bitmask_key);
-                for (int button = BTN_TRIGGER_HAPPY1; button <= BTN_TRIGGER_HAPPY40 && !has_joystick_axes_or_buttons; button++)
-                        has_joystick_axes_or_buttons = test_bit(button, bitmask_key);
-                for (int button = BTN_DPAD_UP; button <= BTN_DPAD_RIGHT && !has_joystick_axes_or_buttons; button++)
-                        has_joystick_axes_or_buttons = test_bit(button, bitmask_key);
+                for (int button = BTN_JOYSTICK; button < BTN_DIGI; button++)
+                        if (test_bit(button, bitmask_key))
+                                num_joystick_buttons++;
+                for (int button = BTN_TRIGGER_HAPPY1; button <= BTN_TRIGGER_HAPPY40; button++)
+                        if (test_bit(button, bitmask_key))
+                                num_joystick_buttons++;
+                for (int button = BTN_DPAD_UP; button <= BTN_DPAD_RIGHT; button++)
+                        if (test_bit(button, bitmask_key))
+                                num_joystick_buttons++;
         }
-        for (int axis = ABS_RX; axis < ABS_PRESSURE && !has_joystick_axes_or_buttons; axis++)
-                has_joystick_axes_or_buttons = test_bit(axis, bitmask_abs);
+        for (int axis = ABS_RX; axis < ABS_PRESSURE; axis++)
+                if (test_bit(axis, bitmask_abs))
+                        num_joystick_axes++;
 
         if (has_abs_coordinates) {
                 if (has_stylus || has_pen)
@@ -235,9 +240,9 @@ static bool test_pointers(sd_device *dev,
                         is_abs_mouse = true;
                 else if (has_touch || is_direct)
                         is_touchscreen = true;
-                else if (has_joystick_axes_or_buttons)
+                else if (num_joystick_buttons > 0 || num_joystick_axes > 0)
                         is_joystick = true;
-        } else if (has_joystick_axes_or_buttons)
+        } else if (num_joystick_buttons > 0 || num_joystick_axes > 0)
                 is_joystick = true;
 
         if (has_mt_coordinates) {
@@ -262,6 +267,34 @@ static bool test_pointers(sd_device *dev,
         if (is_mouse && id->bustype == BUS_I2C)
                 is_pointing_stick = true;
 
+        /* Joystick un-detection. Some keyboards have random joystick buttons
+         * set. Avoid those being labeled as ID_INPUT_JOYSTICK with some heuristics.
+         * The well-known keys represent a (randomly picked) set of key groups.
+         * A joystick may have one of those but probably not several. And a joystick with less than 2 buttons
+         * or axes is not a joystick either.
+         * libinput uses similar heuristics, any changes here should be added to libinput too.
+         */
+        if (is_joystick) {
+                static const unsigned int well_known_keyboard_keys[] = {
+                        KEY_LEFTCTRL, KEY_CAPSLOCK, KEY_NUMLOCK, KEY_INSERT,
+                        KEY_MUTE, KEY_CALC, KEY_FILE, KEY_MAIL, KEY_PLAYPAUSE,
+                        KEY_BRIGHTNESSDOWN,
+                };
+                size_t num_well_known_keys = 0;
+
+                if (has_keys)
+                        for (size_t i = 0; i < ELEMENTSOF(well_known_keyboard_keys); i++)
+                                if (test_bit(well_known_keyboard_keys[i], bitmask_key))
+                                        num_well_known_keys++;
+
+                if (num_well_known_keys >= 4 || num_joystick_buttons + num_joystick_axes < 2) {
+                        log_device_debug(dev, "Input device has %zd joystick buttons and %zd axes but also %zd keyboard key sets, "
+                                         "assuming this is a keyboard, not a joystick.",
+                                         num_joystick_buttons, num_joystick_axes, num_well_known_keys);
+                        is_joystick = false;
+                }
+        }
+
         if (is_pointing_stick)
                 udev_builtin_add_property(dev, test, "ID_INPUT_POINTINGSTICK", "1");
         if (is_mouse || is_abs_mouse)