[MIR] libsdl2

Good point! It would be nice to see us switch main over.

In terms of blockers, this needs a team bug subscriber. And it needs a MIR for tslib too.

But besides that, it looks pretty good. sdl1.2 was already in main, this has few bugs, is well maintained in Debian, we're in sync with Debian. It doesn't use a symbols file, but does explicitly use -V with 2.0.0, which makes sense.

I know that sdl1.2 is already in main, but I figure it would be nice to get security opinion on sdl2, so assigning to ubuntu-security for a quick look or at least a statement saying it doesn't need a quick look.

Oh, looking over my comment, I see that I didn't mention that it has tests that aren't run. It seems like it has quite a few test files; is there a reason they can't be run during build?

@damage3025,

you changed the status from Incomplete but you did not answer mterry's questions. Incomplete means that the bug is awaiting further info.

Note server team would be happy to see this too so that qemu could be switched over from libsdl1.2.

What is the status of this MIR? Is it still needed?

Unfortunately, the security team lost track of this MIR so I'm trying to understand if it is something we need to get to before 16.10. Thanks!

There's a typo in comment #5. I meant to type 15.10 instead of 16.10.

s/15.10/16.04

Ping? The new libsdl2 is now on xenial

This is fixed now?

libsdl2 is still in universe in yakkety; I suspect we cannot get to this request in time for 16.10 release.

Thanks

On the motivation:
Initially this was described as
  "So it's time to replace libsdl1.2 with libsdl2"

Given that there are 400 effective and 327 build depends this certainly will be no "replacement".
Like Serge said in comment #4, some packages might benefit and do the switch - but others might not for quite a while - yet having libsdl2 in main might help to drop some delta and more so not be suddenly blocked once upstream and Debian moves.
It is more:
  "So it's time to allow packages migrating to adopt libsdl2"

On the open questions:
1. MIR for tslib
2. Tests not ran on build

## 1. MIR for tslib ##

tslib is no more part of the dependencies that need to be resolved.
But libsndio-dev is.
for i in $(apt-cache show libsdl2-dev libsdl2-doc libsdl2-2.0-0 | awk '/^Depe/ {gsub("Depends: ",""); gsub(", "," "); gsub("\\(.*\\)",""); print $0;}' | xargs); do if ! apt-cache policy $i| grep 500 | grep -q main; then echo $i; apt-cache policy $i| grep 500; fi; done
libsdl2-2.0-0
     2.0.5+dfsg1-2ubuntu3 500
        500 http://archive.ubuntu.com/ubuntu artful/universe amd64 Packages
libsndio-dev
 *** 1.1.0-3 500
        500 http://archive.ubuntu.com/ubuntu artful/universe amd64 Packages

But that dependency is only from libsdl2-dev only which could be ok since we now can keep build-depends out of main and that is all it is used for.
Can we also make only "some binaries of a source" to appear in main?
Otherwise one would need a MIR for "sndio" (which fortunately would have no further dependencies on its own).

## 2. Tests not ran on build ##

Those are no "common" tests like for a "make check" or anything driven by dh_autotest usually.
Also (naturally) many of those tests need sound or graphic devices to work.
One could add the tests as a dep8 test if that is needed, but it needs more work especially if one would want to enable the graphic and audio checks.
I polled upstream, but got no reply with more details on the tests.

So I checked if I could brute force the tests and could get it to work with a subset of tests and one patch:
/usr/include/SDL2/SDL_stdinc.h:265:20: error: variably modified ‘SDL_dummy_size’ at file scope
        typedef int SDL_dummy_ ## name[(x) * 2 - 1]
                    ^
testatomic.c:106:1: note: in expansion of macro ‘SDL_COMPILE_TIME_ASSERT’
 SDL_COMPILE_TIME_ASSERT(size, CountTo>0); /* check for rollover */
=> That is no more a valid way, disabling the line for now.

# install libsdl2-dev via d/t/control
# make unpacked source available
cd test
./autogen
./configure
make
# then run the subset of tests that work in the build env

I attach a list of working and failing tests, but would not recommend to add those - this would be more an upstream effort to make it into a valid "make check" if that is reasonable (for them).
Surely one could start with a fake window server like in the qt tests, but you see where this is going.

For now the reason the tests are not run is explained, and the way to a dep8 is clear but quite some effort.

##

Setting to new as the questions that were asked before got answered.

Addtional feedback on the tests from IRC on #SDL
That confirms my analysis of being no common "make check" tests and that we might be able to use a few for dep8, but nothing like a real test in the way build time tests usually are used.

[09:49] <cpaelzer> Hi, I'm wondering about the tests in the test subdir of libsdl2
[09:50] <cpaelzer> are they really tests - they seem to be more examples
[09:50] <cpaelzer> If they are tests I'm looking for a way to "run" them e.g. to verify a build
[09:50] <cpaelzer> but reading through the makefiles so far indicates that this isn't meant like a classic "make check"
[09:51] <cpaelzer> I only found libSDL2_test.a built by default but don't know what to do with that either
[09:51] <cpaelzer> If one can share a few details let me know
[10:59] <demonicmaniac3> cpaelzer: they aren't automated tests you can use in a build system to check
[10:59] <demonicmaniac3> they all require manual closing and playing around with input manually and what not
[11:00] <demonicmaniac3> the only thing you can check is sample application not crashing due to missing libraries or what not
[11:00] <demonicmaniac3> by starting and killing with quit and checking exit status
[11:01] <DarkMio> cpaelzer: so it is no unit tests but a "hey developer, before you waste hundreds of hours, try if this stuff works for your system, kthxbye."
[11:03] <cpaelzer> ok thats how I felt looking at it DarkMio
[11:03] <cpaelzer> thanks for the info
[11:03] <cpaelzer> also thanks demonicmaniac3
[11:03] <cpaelzer> I got a few running which could be used as post build verifiers, but as you said no usual automated unit tests
[11:04] <DarkMio> also I believe adding now unit tests to SDL would have no real benefit, since the core code basis is pretty much as stable as it gets

Qemu in Debian now switched over to SDL2, and while there are issues this will be extra maintenance effort. Might even be the same for more packages and we might get more component mismatches soon.

I outlined before that:
- the tests it has are not meant to run on build, see comment #11
- there should be no runtime dependency on libsdl2-dev (so that can stay in universe and therfore eliminate the need for MIR of libsndio-dev)
- tslib dependency is gone (at least when I checked in artful)

What is missing is:
- a Team subscriber (Desktop as it is UI?, it is very central lib so maybe also Foundations)?
- A security ack (there was no nack before, but things might have changed so please recheck)

For the latter being the next step I assign to Tyler to redirect as needed.

Oh I see sec team is already assigned, so that should be ok

Ping on this - how is this making progress or is it being lost?

If we'd get the MIR done to the end of 18.04 I would like to move qemu to use it very early in 18.10.

It is next in the priority queue (if it doesn't get bumped), but hasn't been started yet.

FYI on qemu, at least for that we don't need sdl2 anymore.
This went on and switched to gtk3 and libvte which both are in main already.

There still is plenty of reason for sdl2 still, but qemu is off that list now.

besides the security review, it would would be good to know which packages in main are migratable to libsdl2, and when libsdl1.2 can be demoted.

Since qemu is off the list it looks like the primary motivation for this MIR is now gone - as such, the security team proposes we close this MIR and then if another team still wants libsdl2 in main, they should file a new MIR.

Marking the bug as Incomplete based on Alex' question. Unsubscribing ubuntu-security since there is nothing to do at this team. If a team steps up for non-security maintenance, please feel free to resubscribe the security team.

[Expired for libsdl2 (Ubuntu) because there has been no activity for 60 days.]

Re-awakening this as qemu considers to provide SDL2 based backend (not sdl1) as alternative to gtk.
So the qemu driven demand is coming back into play.

I also had a few community people ask for it.

libdsl1.2 is out of main since Bionic.
This would bring in libsdl2 into groovy (or later).
So the old questions for "transitions" are no more important.

Stats of the day:
dep-sdl2: 179
b-dep-sdl2: 154
dep-sdl1.2: 379
b-dep-sdl1.2: 278

That means ~50 active packages switched over, the rest stayed as is.
But it is no more a matter of duplication since sdl1 is demoted quite a while now.

I'll put this back on security where it was before to evaluate.
@Security - if you end up Nacking I might still be able to provide it in qemu, but as an optional universe-level binary package.

Ping, I know the queue is full and resources scarce but we are really waiting for this to have at least one release (21.10) out in the field before hitting an LTS (22.04) with it.

I reviewed libsdl2 2.0.14+dfsg2-3 as checked into impish. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libsdl2 is a replacement for libsdl1.2 and provides abstractions over
access to video devices, audio, and input devices, often used by games or
game engines and other applications wishing to provide user interfaces.

- CVE History:
  - 16 historical CVEs over a large code-base written in C
    - I expected more to be honest so this is encouraging
- Build-Depends
  - Large list of depends, none out of the ordinary or sensitive
- No pre/post inst/rm scripts
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- 1 binary in PATH from the libsdl2-dev package:
  - -rwxr-xr-x root/root 1307 2021-01-19 01:05 ./usr/bin/sdl2-config
  - this is equivalent of pkg-config and is used to get cflags/ldflags etc
    when compliling and application against libsdl2
- No sudo fragments
- No polkit files
- No udev rules
- No cron jobs
- unit tests / autopkgtests
  - No unit tests run during build that I could see but autopkgtests
    include a basic test to ensure compiling against libsdl2-dev works via
    both cmake and using pkg-config directly
- Build logs:
  - Lots of doxygen related noise in the build logs but nothing of note.

- No Processes spawned
- Memory management is all manual
  - libsdl2 is written in C so all memory management is done manually,
    however looking over a sample of the code this looks relatively careful
    and defensive. There were some instances where callers would not check
    for a NULL return value on memory allocation (which could occur when no
    memory is available), however this likely would only lead to a crash on
    a NULL ptr dereference in subsequent code and hence a denial-of-service
    but given these cases occur when low on memory anyway the impact is not
    much different so they are not too concerning.
- File IO
  - Open joystick device-files etc and these are names are detected by
    walking sysfs etc so are safe from untrusted input
- Logging appears to be careful to avoid format string vulns etc
- Uses lots of environment variables to allow to specify various device
  names or even location of configuration files etc. Whilst they might be
  able to be abused by the user of the library, since libsdl2 is a library
  it does not have elevated privileges to cause any effect as a result so
  this is generally not likely to be an issue.
- Lots of use of privileged functions (e.g. ioctl()) but this is expected
  and again not likely able to cross any security boundary
- No use of cryptography / random number sources etc
- No use of temp files
- No obvious use of networking
- No use of WebKit
- No use of PolicyKit

- No significant cppcheck results
- A large number of issues reported by Coverity, most are likely false
  positives or perhaps of low impact, again since the use of libsdl2 does
  not cross a privilege boundary

For a very large library written in C, libsdl2 is in better shape than I
had feared. The extensive use of manual memory management and the handling
of various APIs and devices means there would be a lot of scope for erro...

Thanks for the review Alex!

SDL as qemu backend is in many cases inferior to GTK which we continue to use and recommend as default. But at the same time there are over and over some edge cases where SDL would be beneficial to users. For those we wanted and will be able to enable it now - that will be great towards 22.04.

Starting Build and Test with this enabled to ensure that the old assumptions still hold true in 2022.

FYI I added the package subscription while we intend to complete this to not miss it later.

BTW it might be time for the dependency stats of the day on this topic comparing v1 and v2:

Mid 2017:
dep-sdl1.2: 400
b-dep-sdl1.2: 327

Mid2020
dep-sdl2: 179
b-dep-sdl2: 154
dep-sdl1.2: 379
b-dep-sdl1.2: 278

Now (mid 2021)
dep-sdl2: 180
b-dep-sdl2: 161
dep-sdl1.2: 301
b-dep-sdl1.2: 270

So to be clear, this still will not allow sdl1 to be removed - but it was already demoted in >=Focal and we will have "only one sdl in main" after this.

As crazy as it seems after all the time we can now enable SDL without pulling it into main (as we have done some splits of runtime modules and -gui components in the past years).
I'll first try to implement it that way.

After a few trials we will give it a show in qemu-system-gui for now which means we will promote it as planned (Staying close to Debian which is +1 on my SDL reenabling there as well).
If it turns out too buggy we will remove it, it it is (somewhat expected) great for some but misleading for others we might move it to a suggested qemu-system-gui-extra package.

Setting fix committed as the qemu that pulls this in was uploaded to impish.

Override component to main
libsdl2 2.0.14+dfsg2-3 in impish: universe/misc -> main
libsdl2-2.0-0 2.0.14+dfsg2-3 in impish amd64: universe/libs/optional/100% -> main
libsdl2-2.0-0 2.0.14+dfsg2-3 in impish arm64: universe/libs/optional/100% -> main
libsdl2-2.0-0 2.0.14+dfsg2-3 in impish armhf: universe/libs/optional/100% -> main
libsdl2-2.0-0 2.0.14+dfsg2-3 in impish i386: universe/libs/optional/100% -> main
libsdl2-2.0-0 2.0.14+dfsg2-3 in impish ppc64el: universe/libs/optional/100% -> main
libsdl2-2.0-0 2.0.14+dfsg2-3 in impish riscv64: universe/libs/optional/100% -> main
libsdl2-2.0-0 2.0.14+dfsg2-3 in impish s390x: universe/libs/optional/100% -> main
libsdl2-dev 2.0.14+dfsg2-3 in impish amd64: universe/libdevel/optional/100% -> main
libsdl2-dev 2.0.14+dfsg2-3 in impish arm64: universe/libdevel/optional/100% -> main
libsdl2-dev 2.0.14+dfsg2-3 in impish armhf: universe/libdevel/optional/100% -> main
libsdl2-dev 2.0.14+dfsg2-3 in impish i386: universe/libdevel/optional/100% -> main
libsdl2-dev 2.0.14+dfsg2-3 in impish ppc64el: universe/libdevel/optional/100% -> main
libsdl2-dev 2.0.14+dfsg2-3 in impish riscv64: universe/libdevel/optional/100% -> main
libsdl2-dev 2.0.14+dfsg2-3 in impish s390x: universe/libdevel/optional/100% -> main
libsdl2-doc 2.0.14+dfsg2-3 in impish amd64: universe/doc/optional/100% -> main
libsdl2-doc 2.0.14+dfsg2-3 in impish arm64: universe/doc/optional/100% -> main
libsdl2-doc 2.0.14+dfsg2-3 in impish armhf: universe/doc/optional/100% -> main
libsdl2-doc 2.0.14+dfsg2-3 in impish i386: universe/doc/optional/100% -> main
libsdl2-doc 2.0.14+dfsg2-3 in impish ppc64el: universe/doc/optional/100% -> main
libsdl2-doc 2.0.14+dfsg2-3 in impish riscv64: universe/doc/optional/100% -> main
libsdl2-doc 2.0.14+dfsg2-3 in impish s390x: universe/doc/optional/100% -> main
22 publications overridden.

However, https://people.canonical.com/~ubuntu-archive/component-mismatches-proposed.html also shows that libsdl2-dev depends on libsndio-dev, which is in universe. So this also needs MIRed, or else libsdl2-dev added to extra-excludes in the seed (which would be unusual but not forbidden).

Yeah this was meant to be an extra-exclude, all of this was going on for so long that I missed to merge this before the promotion. Thanks for the reminder Steve!

sndio resolved via https://code.launchpad.net/~paelzer/ubuntu-seeds/+git/ubuntu/+merge/405363

hmm, the auto update was missing
But it is done:

This bug was fixed in the package qemu - 1:6.0+dfsg-1~ubuntu3

---------------
qemu (1:6.0+dfsg-1~ubuntu3) impish; urgency=medium

 libsdl2 | 2.0.14+dfsg2-3 | impish | source