HappenLee commented on PR #67312:
URL: https://github.com/apache/doris/pull/67312#issuecomment-5473486946

    ## Review 结论
   
     当前版本我不建议合入,应该 Request changes。有两个确定的正确性阻塞问题。
   
     1. P1:posix_spawnp 接到了一个明确“不完整”的实现上
   
     新增的 /tmp/doris-pr-67312/be/src/glibc-compatibility/musl/posix_spawnp.c:15 
调用现有 __posix_spawnx,但这个实现自己就注明只供 clang driver 使用、删掉了一半 musl 逻辑:
   
     /tmp/doris-pr-67312/be/src/glibc-compatibility/musl/posix_spawn.c:1
   
     具体问题包括:
   
     - file_actions 只保存在 args.fa,子进程完全没有执行它。
     - 没处理 open/close/dup2/chdir 等 action。
     - oldmask 未初始化,却在默认路径下传给 pthread_sigmask()。
     - 大部分 spawn attribute 和信号语义被忽略。
     - clone() 失败时返回 -pid,最终通常错误地返回 EPERM,而不是真实 errno。
   
     我用 PR 中的源码做了最小运行探针:添加 dup2 file action 后,posix_spawnp() 返回成功,但重定向 pipe 收到 
0 字节,证明 action 被静默忽略。
   
     而且这些源码被放进 OBJECT library 强制链接,会全局覆盖进程里的 posix_spawn/posix_spawnp,影响范围不只是 
Lance。
   
     2. P1:长 PATH 可以直接造成栈溢出
   
     /tmp/doris-pr-67312/be/src/glibc-compatibility/musl/execvpe.c:31 在子进程栈上创建:
   
     char candidate[path_len + file_len + 1];
   
     最大约为:
   
     PATH_MAX + NAME_MAX + 1 = 4352 bytes
   
     但 /tmp/doris-pr-67312/be/src/glibc-compatibility/musl/posix_spawn.c:65 给 
clone() 的栈只有 1024 字节。
   
     我用约 4090 字节的 PATH 实测,探针直接以 139/SIGSEGV 退出。由于使用了 CLONE_VM | 
CLONE_VFORK,还存在破坏父进程内存的风险。
   
     这是复制代码时丢失了上下文约束:完整 musl 实现分配的是 1024 + PATH_MAX,而 Doris 现有精简版只有 1024。
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to