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

Stack corruption detected when using version 1.21 #2841

Open
2 tasks done
Valkhes opened this issue Dec 1, 2021 · 22 comments · May be fixed by #5153
Open
2 tasks done

Stack corruption detected when using version 1.21 #2841

Valkhes opened this issue Dec 1, 2021 · 22 comments · May be fixed by #5153

Comments

@Valkhes
Copy link

Valkhes commented Dec 1, 2021

  • I have read the FAQ.
  • I have searched in existing issues.

Environment

Describe the bug

I was using for some time scrcpy 1.17 in order to retrieve frames from the device, and decided today to update to the last version. In the previous version, I was able in Python3, following a code a bit like this one https://github.com/DawningW/swy-bot/blob/main/scrcpy.py to:

  • Push the server on my phone
  • Launch the server
  • Forward server port :
  • Then get from the video socket the dummy byte, the device name, and the screen resolution

After that, I was able to receive all my frames from the video socket, and it was working perfectly fine.

In 1.21, I saw that the method to launch the server changed. In 1.17, I was using :

subprocess.Popen(
                ['adb', '-s', device_id, 'shell',
                 'CLASSPATH=/data/local/tmp/scrcpy-server',
                 'app_process', '/', 'com.genymobile.scrcpy.Server 1.17 verbose {} {} {} -1 true - false false 0 false false - -'.format(max_width, bitrate, max_fps)],

But in 1.21, there are now named parameters, and also new one, based on what I saw in the Server.java src code. I modified my code to something like :

['adb', '-s', device_id, 'shell',
                 'CLASSPATH=/data/local/tmp/scrcpy-server',
                 'app_process', '/',
                 'com.genymobile.scrcpy.Server {} log_level=verbose max_size=0 bit_rate=8000000 max_fps=0 lock_video_orientation=-1 tunnel_forward=true send_frame_meta=false control=false display_id=0 show_touches=false stay_awake=false power_off_on_close=false clipboard_autosync=false'], 
                 cwd=SERVER_ROOT)

After launching the server, I'm still able to get access to the dummy byte, the device name, and the screen resolution from the socket, but then I don't receive any frame anymore. I have this error happening :

stack corruption detected (-fstack-protector) Aborted

My question is, is it still possible to retrieve frames this way ? And if it is the case, is there a parameter that is not working like that anymore, and that would explain why I have a problem retrieving the frames from the video socket now ?

Thanks a lot for your time.

@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2021

stack corruption detected (-fstack-protector) Aborted

Is it an error on the host or on the device (sometimes, when app_process fails, it prints Aborted to the console)?
Could you enable ASAN or similar to get more information about the stacktrace (or run in gdb/lldb)?

But in 1.21, there are now named parameters, and also new one, based on what I saw in the Server.java src code. I modified my code to something like…

Note that now you can omit the named parameters (which will use their default values), and only keep the ones you want to change.

@Valkhes
Copy link
Author

Valkhes commented Dec 1, 2021

Hi again,

First of all, thanks a lot for your fast answer. The problem was located on the host.

It was a bit hard in my environment to use gdb, as it is not working well on MacOS it seems. I don't really know ASAN, but it appears that I finally found the problem on my side with your second sentence. I had to remove the parameter :
lock_video_orientation=-1

It seems that the parameter name was correct based on the Server switch cases, but it was breaking everything on my side :

case "lock_video_orientation": int lockVideoOrientation = Integer.parseInt(value); options.setLockVideoOrientation(lockVideoOrientation); break;

I don't really know what can be the cause of this problem, as -1 appears to be the default value in Options.java, but removing it saved my problem.

I don't know if you want to close this issue, or if you want to investigate on this problem.

@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2021

-1 should be handled correctly on the server side, something is probably wrong in the caller side (your client code).

'com.genymobile.scrcpy.Server {} log_level=verbose max_size=0 bit_rate=8000000 max_fps=0 lock_video_orientation=-1 tunnel_forward=true send_frame_meta=false control=false display_id=0 show_touches=false stay_awake=false power_off_on_close=false clipboard_autosync=false'

Is it expected that your string contain {} but no .format()?

@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2021

Or maybe something related to a maximum string length in your client? If you remove other parts but keep lock_video_orientation=-1, does it work?

@Valkhes
Copy link
Author

Valkhes commented Dec 1, 2021

Is it expected that your string contain {} but no .format()?

Yes, I was trying to simplify the code to help reading it for the issue by removing the variables, I just forgot one

Or maybe something related to a maximum string length in your client? If you remove other parts but keep lock_video_orientation=-1, does it work?

It seems it was this problem. By removing other parameters it is launching without problem. I guess I just have to cut the string on my side :)

Thanks a lot for the explanation about the parameters. This is way easier now that it is named and that some of them are not required, I don't had to check everything in the code so it is a great addition :)

I'm closing the issue as it is now resolved.

@Valkhes Valkhes closed this as completed Dec 1, 2021
@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2021

Could you check in adb logcat if there are any errors when you start with lock_video_orientation=-1?

@Valkhes
Copy link
Author

Valkhes commented Dec 1, 2021

Ok, I was maybe a bit fast, as it seems the string was not the problem

Please find attached the result of my adb logcat com.genymobile.scrcpy.Server:V

I'm not experienced enough with adb, but I found my stack corruption error in the log, so it seems it might be useful for you.

logcat.txt

I reopen the issue as it may be a real problem

@Valkhes Valkhes reopened this Dec 1, 2021
@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2021

12-01 17:29:00.188 25359 25359 W System  : ClassLoader referenced unknown path: /data/local/tmp/scrcpy-server.jar
12-01 17:29:00.193 25359 25359 E appproc : ERROR: could not find class 'com.genymobile.scrcpy.CleanUp'
12-01 17:29:00.193 25359 25359 F app_process: thread.cc:2361] No pending exception expected: java.lang.ClassNotFoundException: com.genymobile.scrcpy.CleanUp

This has probably happen when /data/local/tmp/scrcpy-server.jar did not exist on the device (don't forget that it is always deleted automatically).

12-01 17:29:01.385 25341 25386 I ACodec  : reconfigEncoder4OtherApps
12-01 17:29:01.385 25341 25386 F libc    : stack corruption detected (-fstack-protector)
12-01 17:29:01.385 25341 25386 F libc    : Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 25386 (CodecLooper), pid 25341 (main)
12-01 17:29:01.458 25392 25392 I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstone
12-01 17:29:01.459  3823  3823 I tombstoned: received crash request for pid 25386
12-01 17:29:01.460 25392 25392 I crash_dump64: performing dump of process 25341 (target tid = 25386)
12-01 17:29:01.476 25392 25392 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
12-01 17:29:01.476 25392 25392 F DEBUG   : Build fingerprint: 'samsung/a40eea/a40:11/RP1A.200720.012/A405FNXXU3CUD3:user/release-keys'
12-01 17:29:01.476 25392 25392 F DEBUG   : Revision: '4'
12-01 17:29:01.476 25392 25392 F DEBUG   : ABI: 'arm64'
12-01 17:29:01.478 25392 25392 F DEBUG   : Timestamp: 2021-12-01 17:29:01+0100
12-01 17:29:01.478 25392 25392 F DEBUG   : pid: 25341, tid: 25386, name: CodecLooper  >>> app_process <<<
12-01 17:29:01.478 25392 25392 F DEBUG   : uid: 2000
12-01 17:29:01.478 25392 25392 F DEBUG   : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr --------
12-01 17:29:01.478 25392 25392 F DEBUG   : Abort message: 'stack corruption detected (-fstack-protector)'
12-01 17:29:01.479 25392 25392 F DEBUG   :     x0  0000000000000000  x1  000000000000632a  x2  0000000000000006  x3  00000076a1e46fe0
12-01 17:29:01.479 25392 25392 F DEBUG   :     x4  0000000000000000  x5  0000000000000000  x6  0000000000000000  x7  0000000000000030
12-01 17:29:01.479 25392 25392 F DEBUG   :     x8  00000000000000f0  x9  a66957c01c6abd31  x10 0000000000000000  x11 ffffffc0fffffbdf
12-01 17:29:01.479 25392 25392 F DEBUG   :     x12 0000000000000001  x13 0000000061a7a2cd  x14 0016e342450bf280  x15 0000154ffac4aad6
12-01 17:29:01.479 25392 25392 F DEBUG   :     x16 0000007745656948  x17 00000077456352d0  x18 000000769ec2c000  x19 00000000000062fd
12-01 17:29:01.479 25392 25392 F DEBUG   :     x20 000000000000632a  x21 00000000ffffffff  x22 0000007742756d88  x23 00000077436eeb00
12-01 17:29:01.479 25392 25392 F DEBUG   :     x24 00000076a1e48000  x25 00000076a1e47240  x26 00000077436eeb00  x27 00000076a1e48000
12-01 17:29:01.479 25392 25392 F DEBUG   :     x28 0000007742756d88  x29 00000076a1e47060
12-01 17:29:01.479 25392 25392 F DEBUG   :     lr  00000077455e8ca4  sp  00000076a1e46fc0  pc  00000077455e8cd0  pst 0000000000000000
12-01 17:29:01.491 25392 25392 F DEBUG   : backtrace:
12-01 17:29:01.491 25392 25392 F DEBUG   :       #00 pc 0000000000089cd0  /apex/com.android.runtime/lib64/bionic/libc.so (abort+164) (BuildId: 9452bacb546f9fce6f2f7a01c88f996f)
12-01 17:29:01.491 25392 25392 F DEBUG   :       #01 pc 000000000009e08c  /apex/com.android.runtime/lib64/bionic/libc.so (__stack_chk_fail+20) (BuildId: 9452bacb546f9fce6f2f7a01c88f996f)
12-01 17:29:01.491 25392 25392 F DEBUG   :       #02 pc 00000000000c3b10  /system/lib64/libstagefright.so (android::ACodec::reconfigEncoder4OtherApps(android::sp<android::AMessage> const&)+576) (BuildId: 2486ca6f999615d3e33d21562db5984a)
12-01 17:29:01.491 25392 25392 F DEBUG   :       #03 pc 00000065736c6166  <unknown>
12-01 17:29:01.923  4291  4684 W NativeCrashListener: Couldn't find ProcessRecord for pid 25341

Looks like a crash in stagefright (so a bug in the device). But I can't see how passing lock_video_orientation=-1 could change anything. Could you run several times with and without lock_video_orientation=-1 and confirm that it crashes with and doesn't crash without?

@Valkhes
Copy link
Author

Valkhes commented Dec 1, 2021

So, here is what I saw :

  • Running in loop my program is always giving the same result. If it crash one time, it will crash every time with the same code. If it is working, it will work every time.
  • I don't think this is link to the lock_video_orientation=-1 after running some checks, but maybe more link to the number of parameters we are giving, for example, here is a part of my code:
log_level = "verbose"
lock_video_orientation = -1
tunnel_forward = "true"
send_frame_meta = "false"
control = "false"
display_id = 0
show_touches = "false"
stay_awake = "false"
codec_option = "-"
encoder_name = "c2.android.avc.encoder"
power_off_on_close = "false"
clipboard_autosync = "false"
subprocess.Popen(
    ['adb', '-s', self.device_id, 'shell',
     'CLASSPATH=/data/local/tmp/scrcpy-server',
     'app_process', '/',
     'com.genymobile.scrcpy.Server {}'.format(SCREENCOPY_SERVER_VERSION),
     'log_level={}'.format(log_level),
     'bit_rate={}'.format(bitrate),
     'max_fps={}'.format(max_fps),
     'lock_video_orientation={}'.format(lock_video_orientation),
     'tunnel_forward={}'.format(tunnel_forward),
     'send_frame_meta={}'.format(send_frame_meta),
     'control={}'.format(control),
     'display_id={}'.format(display_id),
     'show_touches={}'.format(show_touches),
     'stay_awake={}'.format(stay_awake),
     'power_off_on_close={}'.format(power_off_on_close),
     'clipboard_autosync={}'.format(clipboard_autosync)],
    cwd=SERVER_ROOT)

This is crashing, but as soon as I comment lock_video_orientation :

subprocess.Popen(
    ['adb', '-s', self.device_id, 'shell',
     'CLASSPATH=/data/local/tmp/scrcpy-server',
     'app_process', '/',
     'com.genymobile.scrcpy.Server {}'.format(SCREENCOPY_SERVER_VERSION),
     'log_level={}'.format(log_level),
     'bit_rate={}'.format(bitrate),
     'max_fps={}'.format(max_fps),
     #'lock_video_orientation={}'.format(lock_video_orientation),
     'tunnel_forward={}'.format(tunnel_forward),
     'send_frame_meta={}'.format(send_frame_meta),
     'control={}'.format(control),
     'display_id={}'.format(display_id),
     'show_touches={}'.format(show_touches),
     'stay_awake={}'.format(stay_awake),
     'power_off_on_close={}'.format(power_off_on_close),
     'clipboard_autosync={}'.format(clipboard_autosync)],
    cwd=SERVER_ROOT)

This is working...

Now, let's say I add the parameter encoder_name :

subprocess.Popen(
    ['adb', '-s', self.device_id, 'shell',
     'CLASSPATH=/data/local/tmp/scrcpy-server',
     'app_process', '/',
     'com.genymobile.scrcpy.Server {}'.format(SCREENCOPY_SERVER_VERSION),
     'log_level={}'.format(log_level),
     'bit_rate={}'.format(bitrate),
     'max_fps={}'.format(max_fps),
     #'lock_video_orientation={}'.format(lock_video_orientation),
     'tunnel_forward={}'.format(tunnel_forward),
     'send_frame_meta={}'.format(send_frame_meta),
     'control={}'.format(control),
     'display_id={}'.format(display_id),
     'show_touches={}'.format(show_touches),
     'stay_awake={}'.format(stay_awake),
     'power_off_on_close={}'.format(power_off_on_close),
     'clipboard_autosync={}'.format(clipboard_autosync),
     'encoder_name={}'.format(encoder_name)],
    cwd=SERVER_ROOT)

This is crashing again with the same error... And for example, if I comment some other parameter, such as the log level and the bit_rate :

subprocess.Popen(
    ['adb', '-s', self.device_id, 'shell',
     'CLASSPATH=/data/local/tmp/scrcpy-server',
     'app_process', '/',
     'com.genymobile.scrcpy.Server {}'.format(SCREENCOPY_SERVER_VERSION),
     #'log_level={}'.format(log_level),
     #'bit_rate={}'.format(bitrate),
     'max_fps={}'.format(max_fps),
     #'lock_video_orientation={}'.format(lock_video_orientation),
     'tunnel_forward={}'.format(tunnel_forward),
     'send_frame_meta={}'.format(send_frame_meta),
     'control={}'.format(control),
     'display_id={}'.format(display_id),
     'show_touches={}'.format(show_touches),
     'stay_awake={}'.format(stay_awake),
     'power_off_on_close={}'.format(power_off_on_close),
     'clipboard_autosync={}'.format(clipboard_autosync),
     'encoder_name={}'.format(encoder_name)],
    cwd=SERVER_ROOT)

It is now working.

So, looking at this, I suppose the number of parameters we are sending to the device can make it crash for a reason I don't know. I don't see a link between the parameters I'm removing (why would removing the bit_rate or the log_level would change anything for the encoder_name ?), so I don't see any other way that it's the number of parameters which is causing the problem (or the size of the string, as you said before)

@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2021

'com.genymobile.scrcpy.Server {}'.format(SCREENCOPY_SERVER_VERSION),

Not related, but the version should be passed as a separate parameter (it probably works because in the end adb shell flattens the whole command).

What is the final string length if you concatenate all the arguments split by a space? Maybe there is some 255 limit or something.

@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2021

Also, could you reproduce with another device?

It seems I can't reproduce, even if I put all arguments explicitly.

@Valkhes
Copy link
Author

Valkhes commented Dec 2, 2021

What is the final string length if you concatenate all the arguments split by a space? Maybe there is some 255 limit or something.

Only counting the argument, this would be already 245 characters when using all parameters (as we saw yesterday, some of them such as lock_video_orientation can be removed as I'm using here the default value, so the problem is not impacting me anymore, it's already a good thing for me).

log_level=info bit_rate=8000000 max_fps=0 lock_video_orientation=-1 tunnel_forward=true send_frame_meta=false control=false show_touches=false stay_awake=false power_off_on_close=false clipboard_autosync=false encoder_name=c2.android.avc.encoder

If I'm counting the whole command sent with adb shell, we are reaching 358 characters

adb -s XXXXXXXXXXX shell CLASSPATH=/data/local/tmp/scrcpy-server app_process / com.genymobile.scrcpy.Server 1.21 log_level=info bit_rate=8000000 max_fps=0 lock_video_orientation=-1 tunnel_forward=true send_frame_meta=false control=false show_touches=false stay_awake=false power_off_on_close=false clipboard_autosync=false encoder_name=c2.android.avc.encoder

Also, could you reproduce with another device?

I may be able to do that next week. I don't have another phone right now to try.

@rom1v
Copy link
Collaborator

rom1v commented Dec 2, 2021

Might be a limitation of app_process (or adb shell padding arguments to app_process):

$ printf '/ com.genymobile.scrcpy.Server 1.21 log_level=info bit_rate=8000000 max_fps=0 lock_video_orientation=-1 tunnel_forward=true send_frame_meta=false control=false show_touches=false stay_awake=false power_off_on_close=false clipboard_autosync=false encoder_name=c2.android.avc.encoder' | wc -c
281

without lock_video_orientation=-1:

$ printf '/ com.genymobile.scrcpy.Server 1.21 log_level=info bit_rate=8000000 max_fps=0 tunnel_forward=true send_frame_meta=false control=false show_touches=false stay_awake=false power_off_on_close=false clipboard_autosync=false encoder_name=c2.android.avc.encoder' | wc -c
255

@yume-chan
Copy link
Contributor

Also got this issue on Samsung Galaxy S9 (Android 10), the same crash stack trace.

01-08 21:10:48.983  8338  8355 I ACodec  : reconfigEncoder4OtherApps
01-08 21:10:48.983  8338  8355 F libc    : stack corruption detected (-fstack-protector)
01-08 21:10:48.984  8338  8355 F libc    : Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 8355 (CodecLooper), pid 8338 (main)
01-08 21:10:49.039  8364  8364 E crash_dump64: unknown process state: t
01-08 21:10:49.069  8364  8364 I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstone
01-08 21:10:49.070  1292  1292 I /system/bin/tombstoned: received crash request for pid 8355
01-08 21:10:49.071  8364  8364 I crash_dump64: performing dump of process 8338 (target tid = 8355)
01-08 21:10:49.082  8364  8364 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
01-08 21:10:49.083  8364  8364 F DEBUG   : Build fingerprint: 'samsung/starqltezh/starqltechn:10/QP1A.190711.020/G9600ZHU9FUK5:user/release-keys'
01-08 21:10:49.083  8364  8364 F DEBUG   : Revision: '14'
01-08 21:10:49.083  8364  8364 F DEBUG   : ABI: 'arm64'
01-08 21:10:49.083  8364  8364 F DEBUG   : Timestamp: 2022-01-08 21:10:49+0800
01-08 21:10:49.083  8364  8364 F DEBUG   : pid: 8338, tid: 8355, name: CodecLooper  >>> app_process <<<
01-08 21:10:49.083  8364  8364 F DEBUG   : uid: 2000
01-08 21:10:49.083  8364  8364 F DEBUG   : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr --------
01-08 21:10:49.084  8364  8364 F DEBUG   : Abort message: 'stack corruption detected (-fstack-protector)'
01-08 21:10:49.084  8364  8364 F DEBUG   :     x0  0000000000000000  x1  00000000000020a3  x2  0000000000000006  x3  0000007a0ab81000
01-08 21:10:49.084  8364  8364 F DEBUG   :     x4  0000000000808080  x5  0000000000808080  x6  0000000000808080  x7  0000000000000030
01-08 21:10:49.084  8364  8364 F DEBUG   :     x8  00000000000000f0  x9  dd30231179e08b19  x10 0000000000000001  x11 0000000000000000
01-08 21:10:49.084  8364  8364 F DEBUG   :     x12 fffffff0fffffbdf  x13 0000000061d98d58  x14 0039d9d0f049221a  x15 0000ca169e7eb4e6
01-08 21:10:49.084  8364  8364 F DEBUG   :     x16 0000007aad5548c0  x17 0000007aad530ff0  x18 0000007a0a5ce000  x19 0000000000002092
01-08 21:10:49.084  8364  8364 F DEBUG   :     x20 00000000000020a3  x21 00000000ffffffff  x22 0000007a0ab82020  x23 0000007aad13b6a8
01-08 21:10:49.084  8364  8364 F DEBUG   :     x24 0000007a0ab82020  x25 0000007a0ab812c8  x26 0000007aad13b6a8  x27 0000007a0ab82020
01-08 21:10:49.084  8364  8364 F DEBUG   :     x28 0000007aacd92c70  x29 0000007a0ab810a0
01-08 21:10:49.084  8364  8364 F DEBUG   :     sp  0000007a0ab80fe0  lr  0000007aad4e227c  pc  0000007aad4e22a8
01-08 21:10:49.095  8364  8364 F DEBUG   : 
01-08 21:10:49.095  8364  8364 F DEBUG   : backtrace:
01-08 21:10:49.096  8364  8364 F DEBUG   :       #00 pc 00000000000832a8  /apex/com.android.runtime/lib64/bionic/libc.so (abort+160) (BuildId: d00c50b4798c95f2447d684ed3ea7dcb)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #01 pc 00000000000d13b8  /apex/com.android.runtime/lib64/bionic/libc.so (__stack_chk_fail+20) (BuildId: d00c50b4798c95f2447d684ed3ea7dcb)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #02 pc 00000000000e5240  /system/lib64/libstagefright.so (android::ACodec::reconfigEncoder4OtherApps(android::sp<android::AMessage> const&)+536) (BuildId: 626521096d23fe4ab1fdf19c7b93f835)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #03 pc 00000000000e2c04  /system/lib64/libstagefright.so (android::ACodec::setupAVCEncoderParameters(android::sp<android::AMessage> const&)+2800) (BuildId: 626521096d23fe4ab1fdf19c7b93f835)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #04 pc 00000000000ddd34  /system/lib64/libstagefright.so (android::ACodec::setupVideoEncoder(char const*, android::sp<android::AMessage> const&, android::sp<android::AMessage>&, android::sp<android::AMessage>&)+2832) (BuildId: 626521096d23fe4ab1fdf19c7b93f835)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #05 pc 00000000000d8cf8  /system/lib64/libstagefright.so (android::ACodec::configureCodec(char const*, android::sp<android::AMessage> const&)+7312) (BuildId: 626521096d23fe4ab1fdf19c7b93f835)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #06 pc 00000000000d4868  /system/lib64/libstagefright.so (android::ACodec::LoadedState::onConfigureComponent(android::sp<android::AMessage> const&)+180) (BuildId: 626521096d23fe4ab1fdf19c7b93f835)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #07 pc 00000000000d4688  /system/lib64/libstagefright.so (android::ACodec::LoadedState::onMessageReceived(android::sp<android::AMessage> const&)+540) (BuildId: 626521096d23fe4ab1fdf19c7b93f835)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #08 pc 00000000000f3a20  /system/lib64/libstagefright.so (android::AHierarchicalStateMachine::handleMessage(android::sp<android::AMessage> const&)+260) (BuildId: 626521096d23fe4ab1fdf19c7b93f835)
01-08 21:10:49.096  8364  8364 F DEBUG   :       #09 pc 000000000001a930  /system/lib64/libstagefright_foundation.so (android::AHandler::deliverMessage(android::sp<android::AMessage> const&)+48) (BuildId: 8a68c0993367e3faa25f7ac444fb6770)
01-08 21:10:49.097  8364  8364 F DEBUG   :       #10 pc 000000000001e6e0  /system/lib64/libstagefright_foundation.so (android::AMessage::deliver()+100) (BuildId: 8a68c0993367e3faa25f7ac444fb6770)
01-08 21:10:49.097  8364  8364 F DEBUG   :       #11 pc 000000000001b378  /system/lib64/libstagefright_foundation.so (android::ALooper::loop()+560) (BuildId: 8a68c0993367e3faa25f7ac444fb6770)
01-08 21:10:49.097  8364  8364 F DEBUG   :       #12 pc 00000000000135ec  /system/lib64/libutils.so (android::Thread::_threadLoop(void*)+224) (BuildId: 1d07ca01e00872ed0674078cbbbd979b)
01-08 21:10:49.097  8364  8364 F DEBUG   :       #13 pc 00000000000e6f20  /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+36) (BuildId: d00c50b4798c95f2447d684ed3ea7dcb)
01-08 21:10:49.097  8364  8364 F DEBUG   :       #14 pc 00000000000850c8  /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64) (BuildId: d00c50b4798c95f2447d684ed3ea7dcb)
01-08 21:10:49.102 11224 11224 I io_stats: !@   8,0 r 18764611 582299792 w 3709924 67961352 d 280626 39638872 f 1863602 1872900 iot 7953350 6767499 th 102400 0 0 pt 0 inp 0 0 322520.116

Searched through Android source code and Google, looks like the reconfigEncoder4OtherApps function only exists on Samsung devices, and I can't find the source code for it.


In my test it only happens to OMX.qcom.video.encoder.avc encoder, the other two (c2.android.avc.encoder, OMX.google.h264.encoder) on my device doesn't crash even with a much longer command (>300 characters)


And the character limit of 255 starts from app_process part, for example

> 'app_process / com.genymobile.scrcpy.Server 1.21 log_level=debug max_size=1080 bit_rate=40000 max_fps=0 crop= send_frame_meta=true control=true display_id=0 show_touches=false stay_awake=true encoder_name=OMX.qcom.video.encoder.avc clipboard_autosync=true'.length
< 254

doesn't crash.

> 'app_process / com.genymobile.scrcpy.Server 1.21 log_level=debug max_size=1080 bit_rate=400000 max_fps=0 crop= send_frame_meta=true control=true display_id=0 show_touches=false stay_awake=true encoder_name=OMX.qcom.video.encoder.avc clipboard_autosync=true'.length
< 255

crashes.

@Valkhes
Copy link
Author

Valkhes commented Jan 26, 2022

Hello,
Sorry for the delay to respond on this issue. It was a bit hard to retrieve all my devices to test because of the covid situation (my devices are not all at my house unfortunately so I dont have access to them all the time)

It seems that this problem is happening on all the devices I tested :

  • Samsung Galaxy A40
  • Samsung S9+
  • Pixel 2 XL

It's fine for me as I'm using a lot of default value, so I'm not that impacted. I still have to test based on yume-chan comment if some encoders are better than other. It may indeed be a good test, as I'm trying to use the same encoder for all my devices, which is "OMX.google.h264.encoder" (it seems to be the only encoder I have on all my phone, so I'm using this one to have the same behaviour on all devices about the encoder)

@yume-chan
Copy link
Contributor

Pixel 2 XL

Is the stack trace same? The reconfigEncoder4OtherApps function doesn't seem to exist in official Android source code.

@sbfkcel
Copy link

sbfkcel commented Aug 4, 2022

Consider simplifying parameter names?
There seems to be a limit to the parameter length on Samsung devices

@EasyVector
Copy link

I have also encountered this problem on a samsung device. After I shorten the app_process options, the error of stack corruption detected (-fstack-protector) is gone.

@EasyVector
Copy link

EasyVector commented Nov 17, 2022

Well I did some tests on Samsung devices with Android version of 10 and 11, it turned out that on version 10, using encoders other than OMX.Exynos.AVC.Encoder will not generate the stack error. However on version 11, all of the encoders (including c2.android.avc.encoder, OMX.google.h264.encoder) will produce stack error, when the length of command is over 255. I don't know the reason very well, but it seems to be the bug from Samsung. Just a reminder, in higher version of Android, the encoder in samsang might not work at all when length of command is too long, no matter which encoder you choose.

@EasyVector
Copy link

EasyVector commented Nov 18, 2022

Well, I use ghidra to decompile the libstagefright.so library where the suspicious reconfigEncoder4OtherApps is defined, here is the decompiled code:


/* WARNING: Could not reconcile some variable overlaps */
/* android::ACodec::reconfigEncoder4OtherApps(android::sp<android::AMessage> const&) */

void __thiscall android::ACodec::reconfigEncoder4OtherApps(ACodec *this,sp *param_1)

{
  long lVar1;
  uint uVar2;
  int iVar3;
  char *pcVar4;
  undefined8 *puVar5;
  long *plVar6;
  long lVar7;
  undefined8 local_1c0;
  undefined4 local_1b8;
  undefined4 local_1b4;
  undefined4 uStack432;
  undefined4 uStack428;
  undefined4 uStack424;
  undefined8 local_1a4;
  undefined8 uStack412;
  undefined8 local_194;
  undefined8 uStack396;
  ulong local_184;
  undefined8 uStack380;
  undefined8 local_174;
  undefined8 uStack364;
  undefined8 local_164;
  undefined8 uStack348;
  int local_154;
  undefined8 local_150;
  undefined8 uStack328;
  undefined8 local_140;
  undefined8 uStack312;
  undefined8 uStack304;
  undefined8 uStack296;
  undefined8 local_120;
  undefined8 uStack280;
  undefined8 uStack272;
  undefined8 uStack264;
  undefined8 local_100;
  undefined8 uStack248;
  undefined8 uStack240;
  undefined8 uStack232;
  undefined8 local_e0;
  undefined8 uStack216;
  undefined8 uStack208;
  undefined8 uStack200;
  undefined8 local_c0;
  undefined8 uStack184;
  undefined8 uStack176;
  undefined8 uStack168;
  undefined8 local_a0;
  undefined8 uStack152;
  undefined8 uStack144;
  undefined8 uStack136;
  undefined8 local_80;
  undefined8 uStack120;
  undefined8 uStack112;
  undefined7 uStack104;
  undefined local_61;
  undefined7 uStack96;
  undefined8 uStack89;
  long local_48;
  
  lVar1 = cRead_8(tpidr_el0);
  local_48 = *(long *)(lVar1 + 0x28);
  __android_log_print(4,"ACodec","reconfigEncoder4OtherApps");
  uVar2 = getpid();
  uStack89 = 0;
  uStack96 = 0;
  uStack120 = 0;
  local_80 = 0;
  uStack104 = 0;
  local_61 = 0;
  uStack112 = 0;
  uStack152 = 0;
  local_a0 = 0;
  uStack136 = 0;
  uStack144 = 0;
  uStack184 = 0;
  local_c0 = 0;
  uStack168 = 0;
  uStack176 = 0;
  uStack216 = 0;
  local_e0 = 0;
  uStack200 = 0;
  uStack208 = 0;
  uStack248 = 0;
  local_100 = 0;
  uStack232 = 0;
  uStack240 = 0;
  uStack280 = 0;
  local_120 = 0;
  uStack264 = 0;
  uStack272 = 0;
  uStack312 = 0;
  local_140 = 0;
  uStack296 = 0;
  uStack304 = 0;
  uStack328 = 0;
  local_150 = 0;
  if (*(undefined ***)this != &PTR__ACodec_002fec80) {
    __cfi_slowpath(0x5b6339fcfa1859d1);
  }
  iVar3 = getProcessNameByPid((ACodec *)(ulong)uVar2,uVar2,(char *)&local_150);
  if (iVar3 == 0) {
    pcVar4 = strstr((char *)&local_150,"com.tencent.mm");
    if (pcVar4 == (char *)0x0) goto LAB_001e4100;
    __android_log_print(4,"ACodec","Reconfigure encoder for wechat");
    local_154 = 1;
    puVar5 = *(undefined8 **)param_1;
    __cfi_slowpath(0x7e5cd404d5b7b53,*puVar5);
    android::AMessage::findInt32((char *)puVar5,(int *)"profile");
    if ((local_154 == 8) || (local_154 == 2)) {
      uStack348 = 0;
      local_164 = 0;
      uStack364 = 0;
      local_174 = 0;
      uStack380 = 0;
      local_184 = 0;
      uStack396 = 0;
      local_194 = 0;
      uStack412 = 0;
      local_1a4 = 0;
      uStack428 = 0;
      uStack424 = 0;
      local_1b4 = 0;
      uStack432 = 0;
      local_1c0 = 0x10000006c;
      local_1b8 = 1;
      plVar6 = *(long **)(this + 0xf0);
      lVar7 = *plVar6;
      __cfi_slowpath(0xda41b628d969534c,lVar7);
      pcVar4 = (char *)(**(code **)(lVar7 + 0x38))(plVar6,0x600000d,&local_1c0,0x6c);
      if ((int)pcVar4 == 0) {
        __android_log_print(4,"ACodec","ref: 2, P: 45, B: 1");
        local_1a4 = CONCAT44(local_1a4._4_4_,2);
        uStack432 = 0x2d;
        uStack428 = 1;
        local_184 = local_184 | 0x400000000;
        plVar6 = *(long **)(this + 0xf0);
        lVar7 = *plVar6;
        __cfi_slowpath(0xda41b628d969534c,lVar7);
        pcVar4 = (char *)(**(code **)(lVar7 + 0x40))(plVar6,0x600000d,&local_1c0,0x6c);
      }
      goto LAB_001e4100;
    }
  }
  pcVar4 = (char *)0x0;
LAB_001e4100:
  if (*(long *)(lVar1 + 0x28) == local_48) {
    return;
  }
                    /* WARNING: Subroutine does not return */
  __stack_chk_fail(pcVar4);
}

I don't have much ARM instruction set experience, so I try my best to check the function it called. The one thing I am sure about is the condition *(long *)(lVar1 + 0x28) == local_48 is no longer satisfied, then __stack_chk_fail(pcVar4); is called. But in reconfigEncoder4OtherApps, there is no code that evidently changed the value of lVar1, so I left this behind.

Then I spotted a suspicious function called getProcessNameByPid, this function is defined in libstagefright.so. It appears that it could get the process name of the PID, then reconfigEncoder4OtherApps could compare the process name with the Wechat package name, which is trivial to me, and here is the corresponding decompiled code of getProcessNameByPid.

/* android::ACodec::getProcessNameByPid(int, char*) */

undefined4 __thiscall android::ACodec::getProcessNameByPid(ACodec *this,int param_1,char *param_2)

{
  FILE *__stream;
  size_t sVar1;
  undefined8 in_x4;
  undefined8 in_x5;
  undefined8 in_x6;
  undefined8 in_x7;
  undefined4 uVar2;
  
  FUN_001df2c4(param_2,0xffffffffffffffff,param_2,(ulong)(uint)param_1,in_x4,in_x5,in_x6,in_x7);
  __stream = fopen(param_2,"r");
  if (__stream == (FILE *)0x0) {
    __android_log_print(3,"ACodec","can not found process name");
    uVar2 = 0xffffffff;
  }
  else {
    __cfi_slowpath(0x695b19aca8f9bcf9,fread);
    sVar1 = fread_unlocked(param_2,1,0x200,__stream);
    if (sVar1 == 0) {
      uVar2 = 0xfffffffe;
    }
    else {
      uVar2 = 0;
      param_2[sVar1] = '\0';
    }
    fclose(__stream);
  }
  return uVar2;
}

I think the 0xffffffffffffffff or the function FUN_001df2c4 should be responsible for this stack crash.

Then I repatched the code inside libstagefright.so, changed the code of reconfigEncoder4OtherApps to :

/* android::ACodec::reconfigEncoder4OtherApps(android::sp<android::AMessage> const&) */

void __thiscall android::ACodec::reconfigEncoder4OtherApps(ACodec *this,sp *param_1)

{
  long lVar1;
  __pid_t _Var2;
  long lVar3;
  
  lVar1 = cRead_8(tpidr_el0);
  lVar3 = *(long *)(lVar1 + 0x28);
  __android_log_print(4,"ACodec","reconfigEncoder4OtherApps");
  _Var2 = getpid();
  if (*(undefined ***)this != &PTR_~ACodec_002fec80) {
    __cfi_slowpath(0x5b6339fcfa1859d1);
  }
  if (*(long *)(lVar1 + 0x28) == lVar3) {
    return;
  }
                    /* WARNING: Subroutine does not return */
  __stack_chk_fail(_Var2);
}

Also, there is another function setupVideoEncoder inside libstagefright.so also called this getProcessNameByPid, like this:

  if (((uVar6 & 1) == 0) || ((int)local_27c < 1)) {
    local_27c = getpid();
    if (*(undefined ***)this != &PTR_~ACodec_002fec80) {
      __cfi_slowpath(0x5b6339fcfa1859d1);
    }
    iVar4 = getProcessNameByPid((ACodec *)(ulong)local_27c,local_27c,(char *)&local_180);
    if (iVar4 == 0) {
      __android_log_print(4,"ACodec","app-name : %s",&local_180);
    }
  }

where the getProcessNameByPid is called for printing log, which is trivial to me, so I repatched the code to this:

  if ((((uVar6 & 1) == 0) || (local_27c < 1)) &&
     (local_27c = getpid(), *(undefined ***)this != &PTR_~ACodec_002fec80)) {
    __cfi_slowpath(0x5b6339fcfa1859d1);
  }

Then I used this modified ELF by pushing it to the /data/local/tmp and added the path to LD_LIBRARY_PATH, then the Samsung would use the libstagefright.so I uploaded. After this patch, the MediaCodec could be initialized smoothly.

@EasyVector
Copy link

This is just an experimental thought, and I also don't recommend repatch so file in mobile phone as well. :)
But it at least told us which and where part is wrong....

@yume-chan
Copy link
Contributor

yume-chan commented Feb 2, 2023

I want to have some codec options enabled by default in my client, so I tried to pass options in stdin. The question is whether it's worth making such a fundamental change for only one brand.

https://github.com/Genymobile/scrcpy/compare/dev...yume-chan:scrcpy:feat/stdin-options?expand=1

It doesn't require much code modification though.

The format is like

version:1.25
option:uid=2afc835f
option:log_level=verbose
option:bit_rate=8000000
start

I think it's also possible to send other things in stdin.

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 a pull request may close this issue.

5 participants