Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using SAF link in concat demuxer input file causes segfault in ffmpeg-kit #847

Open
Vorticity-Flux opened this issue Sep 22, 2023 · 9 comments
Assignees
Labels
android Affect Android platform bug Something isn't working fixed-on-development Fixed on the development branch. Not released yet. saf Issue about storage access framework v6.0

Comments

@Vorticity-Flux
Copy link

Vorticity-Flux commented Sep 22, 2023

Description
Using SAF link in concat demuxer input file causes segfault in ffmpeg-kit.

Expected behavior
Concat should be possible for files accessed through SAF.

Current behavior
Segfault in saf_open and saf_close.

To Reproduce
In Android code:
0. Create SAF links from Android content URIs using getSafParameterForRead.

  1. Create concat demuxer input file (in this example mylist.txt) with said SAF links (even one SAF link here is enough to cause segfault):
file saf:1.mp4
file saf:2.mp4
  1. Call ffmpeg concat demuxer like so:
    ffmpeg -y -f concat -safe 0 -protocol_whitelist saf,file,crypto -i mylist.txt -c copy saf:3.mp4
    Segfault will occur.

Logs
Raw error:

02:46:52.913 DEBUG                           A  *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
02:46:52.913 DEBUG                           A  Build fingerprint: '...:user/release-keys'
02:46:52.913 DEBUG                           A  Revision: 'MP1.0'
02:46:52.913 DEBUG                           A  ABI: 'arm64'
02:46:52.913 DEBUG                           A  Timestamp: 2023-09-21 19:46:52.742206726-0000
02:46:52.913 DEBUG                           A  Process uptime: 0s
02:46:52.913 DEBUG                           A  Cmdline: com.org.myservice.process
02:46:52.913 DEBUG                           A  pid: 14779, tid: 14895, name: dmx0:concat  >>> com.org.myservice.process <<<
02:46:52.913 DEBUG                           A  uid: 10220
02:46:52.913 DEBUG                           A  tagged_addr_ctrl: 0000000000000001
02:46:52.913 DEBUG                           A  signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
02:46:52.913 DEBUG                           A  Cause: null pointer dereference
02:46:52.913 DEBUG                           A      x0  0000000000000000  x1  0000000000001656  x2  000000000000003f  x3  0000000000000042
02:46:52.913 DEBUG                           A      x4  0000007b5a26d558  x5  0000007bdffd2c00  x6  00000db200000d7e  x7  00000dd400000e04
02:46:52.913 DEBUG                           A      x8  9514422d968df5fd  x9  9514422d968df5fd  x10 0000000000000000  x11 0000007bddb8f200
02:46:52.913 DEBUG                           A      x12 0000000000000021  x13 00000e0600000e10  x14 00000000000d9350  x15 0000003504316ee0
02:46:52.913 DEBUG                           A      x16 0000007b5dfdc0a8  x17 0000007b5ea91714  x18 0000007b599be000  x19 0000000000000042
02:46:52.913 DEBUG                           A      x20 0000007b5a26e000  x21 0000007b6a372f10  x22 00000000dfb9b0bb  x23 8000000000000000
02:46:52.913 DEBUG                           A      x24 b400007cfffd5610  x25 b400007c20012780  x26 0000007b5d8ade66  x27 0000007b5d8cc8e1
02:46:52.914 DEBUG                           A      x28 00000000b0bbbaae  x29 0000007b5a26d5d0
02:46:52.914 DEBUG                           A      lr  0000007b6a325428  sp  0000007b5a26d5c0  pc  0000007b6a325438  pst 0000000060001000
02:46:52.914 DEBUG                           A  backtrace:
02:46:52.914 DEBUG                           A        #00 pc 0000000000026438  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libffmpegkit.so (saf_close+92) (BuildId: 2b3ddd198561eddc68befc2cd8141f9edb6ebe56)
02:46:52.914 DEBUG                           A        #01 pc 00000000002e966c  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
02:46:52.914 DEBUG                           A        #02 pc 00000000002e98a0  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
02:46:52.914 DEBUG                           A        #03 pc 00000000002ebe48  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so (avio_close+136)
02:46:52.914 DEBUG                           A        #04 pc 00000000002fcfe8  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
02:46:52.914 DEBUG                           A        #05 pc 00000000002fc988  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
02:46:52.914 DEBUG                           A        #06 pc 0000000000453670  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
02:46:52.914 DEBUG                           A        #07 pc 000000000045402c  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
02:46:52.914 DEBUG                           A        #08 pc 0000000000453f1c  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so (av_read_frame+380)
02:46:52.914 DEBUG                           A        #09 pc 000000000005bfb8  /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libffmpegkit.so (BuildId: 2b3ddd198561eddc68befc2cd8141f9edb6ebe56)
02:46:52.914 DEBUG                           A        #10 pc 00000000000b1690  /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+204) (BuildId: 28943f8bb3b7b23557619af9a38223c5)
02:46:52.914 DEBUG                           A        #11 pc 00000000000510ac  /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64) (BuildId: 28943f8bb3b7b23557619af9a38223c5)

ndk-stack:

> ./ndk-stack -sym ~/ffmpeg-kit/android/libs/arm64-v8a -i ~/ffmpeg-kit/dump.txt
********** Crash dump: **********
Build fingerprint: '...:user/release-keys'
#00 0x0000000000026438 /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libffmpegkit.so (saf_close+92) (BuildId: 2b3ddd198561eddc68befc2cd8141f9edb6ebe56)
                                                                                                                                         saf_close
                                                                                                                                         ~/ffmpeg-kit/android/jni/../ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c:581:13
#01 0x00000000002e966c /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
ffurl_closep
~/ffmpeg-kit/src/ffmpeg/libavformat/avio.c:446:15
#02 0x00000000002e98a0 /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
ffurl_close
~/ffmpeg-kit/src/ffmpeg/libavformat/avio.c:463:12
#03 0x00000000002ebe48 /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so (avio_close+136)
                                                                                                                                        avio_close
                                                                                                                                        ~/ffmpeg-kit/src/ffmpeg/libavformat/aviobuf.c:1273:11
#04 0x00000000002fcfe8 /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
open_file
~/ffmpeg-kit/src/ffmpeg/libavformat/concatdec.c:0:9
#05 0x00000000002fc988 /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
concat_read_packet
~/ffmpeg-kit/src/ffmpeg/libavformat/concatdec.c:0:0
#06 0x0000000000453670 /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
ff_read_packet
~/ffmpeg-kit/src/ffmpeg/libavformat/demux.c:571:15
#07 0x000000000045402c /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so
read_frame_internal
~/ffmpeg-kit/src/ffmpeg/libavformat/demux.c:1245:15
#08 0x0000000000453f1c /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libavformat.so (av_read_frame+380)
                                                                                                                                        av_read_frame
                                                                                                                                        ~/ffmpeg-kit/src/ffmpeg/libavformat/demux.c:0:23
#09 0x000000000005bfb8 /data/app/~~w_aCNALGcjlhpfBW0ZSYQg==/com.org.myservice-XnyPNh5HSz3dq8-cXJiG6w==/base.apk!libffmpegkit.so (BuildId: 2b3ddd198561eddc68befc2cd8141f9edb6ebe56)
                                                                                                                                         input_thread
                                                                                                                                         ~/ffmpeg-kit/android/jni/../ffmpeg-kit-android-lib/src/main/cpp/fftools_ffmpeg_demux.c:273:15
#10 0x00000000000b1690 /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+204) (BuildId: 28943f8bb3b7b23557619af9a38223c5)
#11 0x00000000000510ac /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64) (BuildId: 28943f8bb3b7b23557619af9a38223c5)

Environment

  • Platform: Android only
  • Architecture: all
  • Version: v6.0
  • Source branch: main
  • Android Studio version: 2022.3.1
  • Android NDK version: 25.1.8937393

Other
Using of SAF links in concat demuxer input files was previously mentioned in #502

The problem is cased by the following code:

/**
 * Used by saf protocol; is expected to be called from a Java thread, therefore we don't need attach/detach
 */
int saf_open(int safId) {
    JNIEnv *env = NULL;
    (*globalVm)->GetEnv(globalVm, (void**) &env, JNI_VERSION_1_6);
    return (*env)->CallStaticIntMethod(env, configClass, safOpenMethod, safId);
}

The assumption that saf_open/saf_close are only called from a Java thread is wrong as files are read after dmx0:concat thread is forked. Segfault occurs as env is not checked before usage (null reference is returned).

@Vorticity-Flux
Copy link
Author

PR #846 adds AttachCurrentThread/DetachCurrentThread calls to both saf_open and saf_close. It appears to fix the issue.

@tanersener
Copy link
Collaborator

I appreciate you creating a PR address a fix. But, before talking about a fix, we must agree on the problem. And, I don't see a clear analysis about why concat demuxer doesn't work. Try to add something that will help us to validate your theory.

@tanersener tanersener added needs-analysis We don't know that this is. It must be investigated further android Affect Android platform v6.0 labels Sep 22, 2023
@Vorticity-Flux
Copy link
Author

Vorticity-Flux commented Sep 22, 2023

Stack trace decoded with ndk-stack provided in the original problem description indicate where the problem is coming from.

Same results manually reformatted for ease of analysis:

#00 libffmpegkit.so saf_close ~/ffmpeg-kit/android/jni/../ffmpeg-kit-android-lib/src/main/cpp/ffmpegkit.c:581:13
#01 libavformat.so ffurl_closep ~/ffmpeg-kit/src/ffmpeg/libavformat/avio.c:446:15
#02 libavformat.so ffurl_close ~/ffmpeg-kit/src/ffmpeg/libavformat/avio.c:463:12
#03 libavformat.so avio_close ~/ffmpeg-kit/src/ffmpeg/libavformat/aviobuf.c:1273:11
#04 libavformat.so open_file ~/ffmpeg-kit/src/ffmpeg/libavformat/concatdec.c:0:9
#05 libavformat.so concat_read_packet ~/ffmpeg-kit/src/ffmpeg/libavformat/concatdec.c:0:0
#06 libavformat.so ff_read_packet ~/ffmpeg-kit/src/ffmpeg/libavformat/demux.c:571:15
#07 libavformat.so read_frame_internal ~/ffmpeg-kit/src/ffmpeg/libavformat/demux.c:1245:15
#08 libavformat.so av_read_frame ~/ffmpeg-kit/src/ffmpeg/libavformat/demux.c:0:23
#09 libffmpegkit.so input_thread ~/ffmpeg-kit/android/jni/../ffmpeg-kit-android-lib/src/main/cpp/fftools_ffmpeg_demux.c:273:15
#10 libc.so __pthread_start
#11 libc.so __start_thread

input_thread is started by ffmpeg-kit here (not present in the stack trace above):

if ((ret = pthread_create(&d->thread, NULL, input_thread, d))) {

In the input_thread av_read_frame is called. Then according to passed configuration concat demuxer's concat_read_packet is called. After processing text input file concat_read_packet calls open_file on the saf links read from the input file (note that this call happens from the forked input thread):
https://github.com/FFmpeg/FFmpeg/blob/035d187c4d5d96cf6d15237df0c0a20be4e933f1/libavformat/concatdec.c#L756

This results in calls to saf_open and saf_close from the forked thread. Therefore JNI GetEnv returns null env which is dereferenced without checking thus causing Segfault.

return (*env)->CallStaticIntMethod(env, configClass, safOpenMethod, safId);

@tanersener
Copy link
Collaborator

Thanks for the analysis. I agree on most of your findings. I'll mark this issue as bug, since it is an ffmpeg-kit issue.

@tanersener tanersener added bug Something isn't working saf Issue about storage access framework and removed needs-analysis We don't know that this is. It must be investigated further labels Sep 24, 2023
@tanersener tanersener added the fixed-on-development Fixed on the development branch. Not released yet. label Oct 3, 2023
@tanersener
Copy link
Collaborator

Fixed on the development branch. @Vorticity-Flux Thanks for the PR.

Copy link

github-actions bot commented Dec 3, 2023

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Copy link

This issue was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2023
@tanersener tanersener reopened this Apr 10, 2024
@tanersener
Copy link
Collaborator

Re-opened since we haven't released this yet

@tanersener
Copy link
Collaborator

We have decided to retire the ffmpeg-kit project and will no longer publish any new releases. Additionally, all previously released ffmpeg-kit binaries will be removed soon.

The fix for this issue has been applied to the development branch. If you need it, you will need to build the fix yourself. Thank you for your understanding and support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android Affect Android platform bug Something isn't working fixed-on-development Fixed on the development branch. Not released yet. saf Issue about storage access framework v6.0
Projects
No open projects
Development

No branches or pull requests

2 participants