Skip to content

Validate effect bytes before native force-feedback upload - #262

Open
omrzk wants to merge 1 commit into
gvalkov:mainfrom
omrzk:fix/validate-upload-effect-data-261
Open

omrzk wants to merge 1 commit into
gvalkov:mainfrom
omrzk:fix/validate-upload-effect-data-261

Conversation

@omrzk

@omrzk omrzk commented Nov 5, 2026

Copy link
Copy Markdown

_input.upload_effect() calls PyBytes_AsString() and copies from its result even when conversion failed, so a non-bytes argument crashes the interpreter. Check conversion and require enough bytes for struct ff_effect before copying. Complete byte strings (including subclasses and trailing data), ioctl errors, and the InputDevice.upload_effect() serialization path retain their behavior.

Fixes #261.

Validation: all 13 new native-extension tests pass on Python 3.14. On current main, six non-bytes cases terminate subprocesses with SIGSEGV and three short-buffer cases fail; four preservation controls pass. All 20 tests outside the uinput module pass. The full suite has 24 passes and six failures because this runner has no /dev/uinput; the same six fail on clean main. Native build, wheel/sdist build, pylint error check and changed-test Ruff pass. Documentation builds with two missing ecodes_generated warnings. No actual force-feedback/input hardware was tested.

@sezanzeb sezanzeb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The change works for me, it raises a TypeError as excpected via the code sample in #261:

Python 3.14.4 (main, Aug 20 2026, 10:41:58) [GCC 15.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import evdev._input
... 
... evdev._input.upload_effect(0, None)
... 
Traceback (most recent call last):
  File "<python-input-0>", line 3, in <module>
    evdev._input.upload_effect(0, None)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
TypeError: expected bytes, NoneType found
>>> 

pipeline passed

Comment thread tests/test_input.py
pass


@pytest.mark.parametrize("data", [bytes(ff.Effect()), bytes(ff.Effect()) + b"extra", EffectBytes(bytes(ff.Effect()))])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrapping bytes(ff.Effect()) in EffectBytes seems to be redundant

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prints true:

from evdev import _input, device, ff

class EffectBytes(bytes):
    pass

a = EffectBytes(bytes(ff.Effect()))
b = bytes(ff.Effect())
c = EffectBytes(ff.Effect())
print(a)
print(b)
print(c)
print(a == b == c)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is there something I'm missing?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subclass case is intentional: equal payloads still reach the extension as different Python types. The first case covers plain bytes; EffectBytes checks that validation continues to accept bytes subclasses, and the extra-data case preserves trailing-byte acceptance.

You are right that the inner bytes() conversion is redundant: EffectBytes(ff.Effect()) produces the same subclass and payload. I checked all three expressions locally; each reaches ioctl and raises EBADF with fd -1. The subclass wrapper is what adds coverage here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_input.upload_effect() segfaults when the effect data is not bytes

2 participants