Surface with negative pitch no longer allowed since 2.24

[ghost]

I guess it's just a Debian issue since I compiled a game on Debian and everything works if I run it on Mint. I've got a lot of games made with SDL2 and none of the icons or custom cursors work anymore.

Not sure where else to report this if it's not an SDL issue.

[slouken]

@smcv, are you seeing this?

[smcv]

What desktop environment are you using, in what mode (X11 or Wayland), and do you have the SDL_VIDEODRIVER environment variable set?

[smcv]

Custom cursors:

If you install the libsdl2-tests package and run /usr/libexec/installed-tests/SDL2/testcustomcursor, what happens? What I get (on Debian 12 with the GNOME desktop in its default Wayland mode) is: there is a grey window, and when I move the mouse over it, my cursor changes to a white arrow with no shading or other fancy rendering, which is not my default cursor and reminds me of Windows 95. I think that means the custom cursor feature is working correctly?

[edit: It also works the same if I log out from GNOME and log back in to XFCE, which is an X11-only environment.]

The same thing works if I run it with SDL_VIDEODRIVER=wayland (in which case I get a black libdecor titlebar instead of the usual GNOME titlebar).

[smcv]

Window icons:

As far as I know, SDL_SetWindowIcon() is not necessarily expected to work in Wayland, because in Wayland there is no such thing as a "window icon". Instead, applications have an icon, the application is expected to associate itself with a .desktop file (either by setting its window ID to point to the .desktop file, or by setting the StartupWMClass in the .desktop file to reflect its window ID), and whatever is the icon of the .desktop file, that's what will be used.

[edit: I tried installing and running openarena. When I run it as an X11 game, either in XFCE or GNOME/Wayland but without forcing a specific SDL_VIDEODRIVER, I get an icon that looks like https://salsa.debian.org/games-team/ioquake3/-/blob/debian/latest/misc/quake3.png. When I run it in GNOME/Wayland with SDL_VIDEODRIVER=wayland, I get the default/"missing" icon. This seems appropriate for what I expected.]

[smcv]

I've got a lot of games made with SDL2

Please give some specific examples of games that are not behaving as you expect? Preferably games that came from Debian, preferably small, and preferably well-known/exemplary rather than something obscure or known to have problems?

[ghost]

I've tried Cinammon (non-software) and Xfce. The testcustomcursor example works for me.

My code is here in set_window_icon and set_custom_mouse_cursor (I have a C++ layer I've written in between SDL2 and my games called shim): https://github.com/troutsneeze/shim4/blob/main/src/gfx.cpp

The binaries are on http://nooskewl.com. The code in Shim hasn't changed I don't think so Monster RPG 3, Dog-O, 3 Crystals and Tower To Heaven are all games using this code. Dog-O is the only one I recompiled on Debian 12, but they all don't show icons or cursors for me. 3 Crystals may not have a custom mouse cursor but they all have icons and most have a custom mouse cursor.

Is it possible there is a size issue or something, that has happened to me before but I think it was on Windows, where it didn't work if my mouse cursor wasn't a specific size.

EDIT: Openarena shows an icon as well so I have no clue why this is happening all of a sudden. SDL_VIDEODRIVER isn't set.

EDIT: I do have 2 games in debian, both named stax one covered the other up but they are not SDL games.

[ghost]

Actually the 3 Crystals and Tower To Heaven binaries were built on Debian 12 too, as was BooBoo which doesn't show an icon either (not a game per se but it uses the same code.)

[smcv]

The testcustomcursor example works for me

A minimal reproducer that doesn't work for you would probably be useful, ideally in C. If testcustomcursor with its minimal custom cursor does work, and your full game engine with the cursor you're actually using doesn't, then there must be some point along the spectrum between the two where it stops working.

There doesn't seem to be a test program in SDL that calls SDL_SetWindowIcon, but similarly, a small reproducer would probably be useful.

Is it possible there is a size issue or something

You could find out by using the cursor from SDL's test (or anything else that has a custom cursor that works) in your code, or conversely, by modifying SDL's test to use your custom cursor.

[ghost]

Yeah, obviously that would work, but why did this break in the first place is another question, Monster RPG 3 was the first game I made with this code and that was like 2018, so I had hoped someone changed something or knew about some change because this was working code.

[smcv]

For reference, Debian 12 has SDL 2.26.5.

You haven't specified which version of Linux Mint had this working, but if Mint doesn't patch or backport SDL and instead inherits it from Ubuntu, then it will be an older version, most likely 2.0.20 or 2.0.10. dpkg-query -W libsdl2-2.0-0 should tell you.

[smcv]

I've tried Cinammon (non-software) and Xfce

I'm fairly sure those are both X11 environments, so nothing Wayland-related will be happening.

[ghost]

Yes, it's 2.0.20 on the Mint machine.

There are multiple things I would check:

where the calls are made size of icons/cursors colour format

But this seems unlikely to be the reason, however it's all I can think of at the moment on my end.

[smcv]

I had hoped someone changed something or knew about some change

2.0.20 is more than a year older than 2.26.5, and the lower-level libraries in Linux Mint and Debian will similarly have about a year of differences. I don't know a comprehensive list of everything that has changed in either SDL or the operating system during that time (it would be long).

why did this break in the first place is another question

I don't know, but having a reproducer for the bug that isn't an entire game would help us to find out why (and fix it).

[ghost]

I'll try to rip everything out so it's just a window or something and see if it works or not.

[ghost]

icon_cursor_test.zip

Here's a cut down test, no icon or cursor on Debian but it does have them on Mint.

[Kontrabant]

Couple of problems here:

On lines 247 and 261 in your example, SDL_CreateRGBSurfaceFrom() is being passed a negative pitch, which causes it to fail, as the pitch can't be less than 0. I see that you are trying to flip the image vertically, but SDL won't let you do that as it enforces a requirement that the pitch be positive. See:

https://github.com/libsdl-org/SDL/blob/3aba9d44732d6e4538deb2c9e4389951fe5a0de5/src/video/SDL_surface.c#L235C5-L238C6

If I remove the attempt to flip the image, it works, albeit the pointer is upside down.

Also, your program isn't very responsive as you need to call SDL_PollEvent() more than once per frame in the main loop, so instead of:

if (SDL_PollEvent(&sdl_event)) {
            if (sdl_event.type == SDL_QUIT) {
                break;
            }
        }

You want something like:

while (SDL_PollEvent(&sdl_event)) {
            if (sdl_event.type == SDL_QUIT) {
                goto done;
            }
        }

where 'done' jumps out of the loop.

As @smcv mentioned, Wayland doesn't support setting icons programmatically at all, so SDL_SetWindowIcon() is not going to work on a Wayland desktop, no matter what.

[ghost]

Couple of problems here:

On lines 247 and 261 in your example, SDL_CreateRGBSurfaceFrom() is being passed a negative pitch, which causes it to fail, as the pitch can't be less than 0. I see that you are trying to flip the image vertically, but SDL won't let you do that as it enforces a requirement that the pitch be positive. See:

Thanks, that's the fix.

The SDL_PollEvent thing was just a bug with this test so I wouldn't worry about it.

As @smcv mentioned, Wayland doesn't support setting icons programmatically at all, so SDL_SetWindowIcon() is not going to work on a Wayland desktop, no matter what.

Yes I was aware of those desktops, but I'm glad for the clarification that it's a Wayland thing.

[smcv]

On lines 247 and 261 in your example, SDL_CreateRGBSurfaceFrom() is being passed a negative pitch

This looks very similar to #6107, except #6107 dealt with insufficient pitch (overlapping rows) rather than negative pitch (later rows appearing in memory before earlier ones).

The behaviour change was a result of 535fdc3a "Detect and reject nonsense SDL_surface dimensions" back in the 2.24.x cycle, part of #5645. The purpose of that change was to harden against nonsense parameters, primarily so that they can't be used as part of a security vulnerability (an attacker inducing a SDL game to load a crafted, invalid bitmap and as a result, do an out-of-bounds access that can corrupt memory or crash), and secondarily to avoid security researchers reporting out-of-bounds accesses as a security vulnerability in situations where they are not actually attacker-triggerable.

As with #6107, if negative pitch (reading the buffer backwards) is a thing that games genuinely need to do, then it would be possible to relax those checks and make it defined behaviour, perhaps something like:

    if (pitch != 0 && ((size_t)abs(pitch) < minimalPitch)) {
        SDL_InvalidParamError("pitch");

so that overlapping rows (#6107) are still forbidden.

The cost would be that it would become less clear what is or isn't meant to be allowed, and more likely that an attacker can trick a SDL game into crashing or corrupting memory by providing a crafted/invalid bitmap. In particular, requiring the pitch to be non-negative means we can say that SDL_CreateRGBSurfaceFrom() will only read memory from a buffer starting at pixels and continuing for pitch * height bytes. If the pitch is allowed to be negative, then SDL_CreateRGBSurfaceFrom() can read from memory addresses strictly earlier than pixels, which seems likely to break someone's assumptions.

See also 77bcd269 "Allow creating an empty surface with pitch 0" which relaxed the check to allow pitch = 0 (every row occupies the same space), and d496d187 "Document that the pitch value may be zero for surfaces that will be filled in by the application later" which disallowed pitch = 0 except in the special case where pixels = NULL.

[smcv]

Now that we know the root cause, it would probably make sense to retitle this issue to "surface with negative pitch no longer allowed since 2.24" to make it more findable. (In particular this is not Debian-specific.)

[ghost]

Yeah it's a toss up kinda, I had to create a temp buffer for the surface to flip it in my games, however if you're dealing with SDL_Surface a lot then you can load the memory correctly initially, I'm only needing to flip a small icon and cursor so it's not a major issue, I could and might just load them flipped then rework the d3d/gl code that needs it later.