已合并
[bugfix] 重明安全排查问题修改 #62
zhaishangzhao创建于 6月30日
[bugfix] 重明安全排查问题修改 #62
已合并
zhaishangzhao创建于 6月30日
zhaishangzhao
zhaishangzhao
6月30日

1. 修改描述

  • 修改原因: https://gitcode.com/Ascend/msot/issues/62
  • 修改方案: 按照重明安全排查漏洞修复方案进行修改
  • 修改内容: 修改PyLong_AsVoidPtr无指针验证,可能导致非法指针解引用与进程崩溃问题

2. 功能验证


3. 代码检视

  • 要求:
    • 合入功能代码大于 200 行,需要sig会议申报代码检视议题,并在PR中标注会议。
    • committer评估是否需要在sig会议进行代码检视。
    • 参与检视的committer人员名单与检视时间。
    • 大于 1000 行代码原则上不允许合入,需进行备案。
  • 检视committer人员名单与检视时间:

4. 资料修改自检

  • 资料修改:

likedislike
Pull Request已成功合入, 合并人@ascend-robot
(感谢 zhaishangzhao 的贡献)
zhaishangzhaozhaishangzhao
6月30日 关联了issue:[Bug]:修复各个代码仓安全扫描处的相关问题
ascend-robotascend-robot成员
6月30日 添加了label:stat/needs-squash
ascend-robotascend-robot成员
6月30日 添加了label:ascend-cla/yes
ascend-robot
ascend-robot成员
6月30日 评论:

Thanks for your pull-request.
The full list of commands accepted by me can be found at here
You can get sig-info at here


PR Approval Progress

Congratulations! All modules have met the lgtm and approve requirements.

Module Approval Details

module lgtm status approve status
repo-Ascend/mstx wiyr0, zzzsss1234 (2/2) wiyr0 (1/1)

💡 Tip:

  • Committer can comment /approve or /lgtm
  • Commenting /approve implies both code review (lgtm) and intent to merge (approve)

CLA Signature Pass

zsz_wlf, thanks for your pull request. All authors of the commits have signed the CLA. 👍

likedislike
zhaishangzhao
zhaishangzhao
6月30日 评论:

compile

likedislike
ascend-robotascend-robot成员
6月30日 添加了label:ci-pipeline-running
ascend-robot
ascend-robot成员
6月30日 评论:

ascend docs pipeline is running...

likedislike
ascend-robotascend-robot成员
6月30日 添加了label:docs-ci-pipeline-running
ascend-robot
ascend-robot成员
6月30日 评论:

ascend docs pipeline is running...

likedislike
ascend-robot
ascend-robot成员
6月30日 评论:

✅ 跳过 docs ci 检查,没有需要检查的文档文件

likedislike
ascend-robotascend-robot成员
6月30日 删除了label:docs-ci-pipeline-running
ascend-robotascend-robot成员
6月30日 添加了label:docs-ci-pipeline-success
atomgit-bot
atomgit-bot
6月30日 评论:

变更摘要

此PR修复了重明安全排查发现的漏洞:PyLong_AsVoidPtr 在转换 Python 对象时缺少类型校验,可能导致非法指针解引用与进程崩溃。主要改动集中在 python/mstx_api.cpp 文件,对 WrapMstxMarkAWrapMstxRangeStartA 两个函数中的 py_stream 参数处理逻辑进行了加固,增加了类型检查与错误处理,同时伴随部分代码格式调整。

主要改动

  • PyLong_AsVoidPtr 替换为安全的类型校验与转换流程:在 WrapMstxMarkAWrapMstxRangeStartA 中,将原来直接调用 PyLong_AsVoidPtr(py_stream) 的无校验转换,改为先用 PyLong_Check 校验对象是否为整数类型,再通过 PyLong_AsUnsignedLongLong 获取 unsigned long long 值并检查 PyErr_Occurred() 错误状态,最后才转换为 void* 指针。
  • 新增类型错误提示:当 py_stream 不是 PyLong 类型时,通过 PyErr_SetString 设置 PyExc_TypeError 异常,提示 "stream must be an integer (aclrtStream handle) or None",提升调用方的调试体验。
  • 代码风格统一:对 GILCtrl 类、ParseArgs 函数签名以及多个函数定义的指针声明风格(Type*Type *)、大括号换行位置、kwlist 数组初始化格式进行了统一调整。
likedislike
ascend-robot
ascend-robot成员
6月30日 评论:

✅ 跳过 docs ci 检查,没有需要检查的文档文件

likedislike
ascend-robotascend-robot成员
6月30日 删除了label:ci-pipeline-running
ascend-robotascend-robot成员
6月30日 添加了label:ci-pipeline-passed
ascend-robot
ascend-robot成员
6月30日 评论:
流水线 PR-pipeline_mstx#107 [ commitID:8f945419 ] 已完成
>>>代码风格自动修复执行成功(无修复内容)
阶段 任务名 状态 详情
编译构建 Build_mstx_arm >>>
Build_mstx_x86 >>>
恶意代码检查 Antipoison_mstx >>>
编码安全与规范检查 CodeCheck_mstx >>>
PreCommit_gitcode >>>
开源片段检查 SCA_mstx >>>
开发者测试 UT_mstx >>>
PreSmoke_mstx >>>
流水线 PR-pipeline_mstx >>>
此流水线已支持下列评论快捷指令,仅PR创建者和白名单成员评论有效
  • compile : 运行流水线
  • retry : 重试流水线所有失败子任务
  • retry <任务名> : 仅重试指定失败子任务
  • stop : 停止流水线
likedislike
ascend-robot
ascend-robot成员
6月30日 评论:
流水线 PR-pipeline_mstx#107 [ commitID:8f945419 ] 已完成
>>>代码风格自动修复执行成功(无修复内容)
阶段 任务名 状态 详情
编译构建 Build_mstx_arm >>>
Build_mstx_x86 >>>
恶意代码检查 Antipoison_mstx >>>
编码安全与规范检查 CodeCheck_mstx >>>
PreCommit_gitcode >>>
开源片段检查 SCA_mstx >>>
开发者测试 UT_mstx >>>
PreSmoke_mstx >>>
流水线 PR-pipeline_mstx >>>
此流水线已支持下列评论快捷指令,仅PR创建者和白名单成员评论有效
  • compile : 运行流水线
  • retry : 重试流水线所有失败子任务
  • retry <任务名> : 仅重试指定失败子任务
  • stop : 停止流水线
likedislike
atomgit-bot
atomgit-bot
6月30日 评论:

代码审查

我已经仔细审查了整个 diff 文件和相关的周围代码。让我总结分析:

变更分析

这个 PR 将 py_stream 的指针转换从直接使用 PyLong_AsVoidPtr(无验证)改为:

  1. 先用 PyLong_Check() 做类型校验
  2. 再用 PyLong_AsUnsignedLongLong() 做值域转换
  3. 最后通过 PyErr_Occurred() 检查溢出错误
  4. reinterpret_cast<void*> 转为指针

同时还有大量纯格式变更(括号位置、空格、指针声明的 * 位置)。

详细排查结果

正确性审查

  • aclrtStream 定义为 typedef void*ms_tools_ext.h:43),unsigned long longvoid*reinterpret_cast 在 64 位平台上正确保持位模式
  • PyLong_Check 后再调用 PyLong_AsUnsignedLongLong 不会因非整数类型失败
  • PyErr_Occurred() 检查可正确捕获负数和超大值导致的 OverflowError
  • 错误路径正确返回 nullptr 并保持 Python 异常状态

安全性审查

  • 旧代码 PyLong_AsVoidPtr 无指针验证,传入非整数或非法值可能导致未定义行为 → 已修复
  • 新增的 PyLong_CheckPyErr_Occurred 检查构成完整输入校验链

已注意但确认为非问题的点

  • PyLong_Checkbool 返回 true(Python 中 boolint 子类),此为 Python 语言特性,且旧代码行为一致
  • message 变量在 WrapMstxMarkA 中未显式初始化即传入 ParseArgs,但 ParseArgs 首行即设为 nullptr,无 UB,且为已有行为
  • 代码块在 WrapMstxMarkAWrapMstxRangeStartA 中重复,属于可维护性问题但非正确性缺陷,不报告

检查结论:本次变更未发现任何正确性、安全性、可靠性或破坏性变更问题。 这是一次纯粹的防御性安全加固,改动逻辑正确。

Closing Summary

  • 审查文件python/mstx_api.cpp — 已审查,无问题
  • 发现数量:P0: 0, P1: 0, P2: 0, P3: 0
  • 整体风险评估:低风险。变更引入了对 py_stream 参数的类型校验和错误检查,将之前无保护的 PyLong_AsVoidPtr 调用替换为三阶段安全转换(类型检查 → 值域转换 → 错误检查),有效杜绝了非法指针解引用风险。格式变更无语义影响。

⚠️ 已识别出整体风险,但无法提取行内评论,请参考整体评估。

likedislike
流栎
流栎成员
7月1日 评论:

/lgtm

likedislike
wiyr0wiyr0成员
7月1日 关联了里程碑:MindStudio 26.1.0
wiyr0
wiyr0成员
7月1日 评论:

/approve

likedislike
ascend-robotascend-robot成员
7月1日 添加了label:approvedlgtm
ascend-robotascend-robot成员
7月1日 合入了pull request