Skip to content

Comments

chore: Update compiler flags for security enhancements#418

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wangrong1069:pr0131
Feb 2, 2026
Merged

chore: Update compiler flags for security enhancements#418
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wangrong1069:pr0131

Conversation

@wangrong1069
Copy link
Contributor

As title

Log: Update compiler flags for security enhancements
Bug: https://pms.uniontech.com/bug-view-342665.html

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @wangrong1069, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@github-actions
Copy link

github-actions bot commented Feb 2, 2026

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "    HOMEPAGE_URL \"https://github.com/linuxdeepin/deepin-editor\"",
            "line_number": 8,
            "rule": "S35",
            "reason": "Url link | 9c6a12da6f"
        }
    ]
}

As title

Log: Update compiler flags for security enhancements
Bug: https://pms.uniontech.com/bug-view-342665.html
@github-actions
Copy link

github-actions bot commented Feb 2, 2026

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "CMakeLists.txt": [
        {
            "line": "    HOMEPAGE_URL \"https://github.com/linuxdeepin/deepin-editor\"",
            "line_number": 8,
            "rule": "S35",
            "reason": "Url link | 9c6a12da6f"
        }
    ]
}

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的主要目的是在 Release 模式下为项目添加编译器和链接器的"加固"(Hardening)选项,以增强程序的安全性和稳定性。整体逻辑是正确的,但在语法逻辑代码质量代码性能/安全方面有一些可以优化的地方。

以下是详细的审查意见和改进建议:

1. 语法逻辑与代码质量

1.1. message() 命令的用法

  • 问题message("Enable build hardening.") 使用的是普通消息模式。在 CMake 配置过程中,普通消息可能会被大量的输出淹没,或者被误认为是普通的状态输出。
  • 建议:建议使用 STATUS 类型,或者为了强调这是一个重要的安全配置,使用 WARNINGAUTHOR_WARNING(虽然叫 WARNING,但常用于高亮显示重要配置信息)。
  • 改进
    message(STATUS "Enable build hardening for Release build.")

1.2. CMAKE_VERBOSE_MAKEFILE

  • 问题:代码中设置了 set(CMAKE_VERBOSE_MAKEFILE ON)。这通常用于调试构建过程,会打印出详细的编译命令。
  • 建议:在 Release 模式下强制开启详细输出通常是不必要的,这会污染构建日志。除非开发者特意需要检查加固标志是否生效,否则建议移除这一行,或者将其移到调试逻辑之外。

1.3. 字符串拼接与变量引用

  • 问题set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${HARDENING_FLAGS}") 这种写法是标准的,但容易导致重复添加(例如如果 CMake 被多次配置)。
  • 建议:CMake 中直接追加字符串通常是可以的,因为 CMakeLists.txt 通常只执行一次。但为了代码的健壮性,可以使用 list(APPEND ...) 逻辑(虽然对于 flags 字符串,直接拼接更常见)。目前的写法是可以接受的,但要注意 HARDENING_FLAGS 是一个整体字符串。

2. 代码安全与加固标志审查

这段代码引入了许多 GCC/Clang 的安全特性,大部分选择是合理的,但有几个细节需要根据项目实际情况调整:

2.1. 优化等级冲突 (-O2)

  • 问题HARDENING_FLAGS 中包含了 -O2
  • 风险:CMake 默认的 Release 模式通常已经设置了优化等级(通常是 -O3)。在这里显式设置 -O2 会覆盖掉默认设置,或者产生冲突(取决于 CMake 版本和顺序),导致性能下降。
  • 建议:不要在 HARDENING_FLAGS 中硬编码优化等级。应该让 CMake 的 CMAKE_BUILD_TYPE=Release 自己管理优化选项(如 -O3-O2)。如果必须指定,建议检查变量是否已包含优化选项。
  • 改进:从 HARDENING_FLAGS 中移除 -O2-g

2.2. 调试信息 (-g)

  • 问题:包含了 -g
  • 建议:虽然 Release 版本带调试符号有利于事后分析崩溃,但这会增加二进制文件体积。如果项目有专门的构建脚本生成带符号的包,这里可以去掉;如果希望 Release 也能直接调试,则保留。建议将其移除,或者单独控制。

2.3. 路径映射 (-ffile-prefix-map)

  • 问题-ffile-prefix-map=${CMAKE_SOURCE_DIR}=.。这个选项用于去除构建路径中的调试信息,有助于构建二进制复现。
  • 风险CMAKE_SOURCE_DIR 如果包含特殊字符或空格,可能会导致编译错误。虽然通常源码目录不含空格,但为了健壮性,建议加上引号保护(虽然在 CMake set 中变量展开通常没问题,但在传递给编译器时需注意)。
  • 建议:目前的写法在大多数情况下是安全的。

2.4. 格式化安全 (-Werror=format-security)

  • 问题-Wformat -Werror=format-security。将格式化字符串警告视为错误。
  • 建议:这是一个非常好的安全实践,能防止格式化字符串漏洞。强烈保留

3. 代码性能

  • 分析-fstack-protector-strong-fstack-clash-protection 会对性能产生极小的影响(通常 < 1%),但换来巨大的安全性提升。在 Release 版本中是完全可接受的。
  • 分析-Wl,-z,relro -Wl,-z,now (RELRO - Relocation Read-Only) 会略微增加程序的启动时间(因为启动时需要解析符号),但能防止 GOT覆写攻击。这也是值得的。

4. 改进后的代码示例

综合以上建议,改进后的代码如下:

if(CMAKE_BUILD_TYPE STREQUAL "Release")
    message(STATUS "Enable build hardening for Release build.")

    # 移除 CMAKE_VERBOSE_MAKEFILE ON,除非调试需要
    
    # 移除 -O2 和 -g,避免覆盖 CMake 默认的 Release 设置或增加不必要的体积
    # 使用引号包裹路径相关的变量,确保安全
    set(HARDENING_FLAGS 
        "-Wdate-time -D_FORTIFY_SOURCE=2 "
        "-ffile-prefix-map=${CMAKE_SOURCE_DIR}=. "
        "-fstack-protector-strong "
        "-fstack-clash-protection "
        "-Wformat -Werror=format-security"
    )

    # 追加标志而不是覆盖,确保不破坏之前的设置
    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${HARDENING_FLAGS}")
    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${HARDENING_FLAGS}")
    
    # 链接器标志
    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-z,relro -Wl,-z,now")
endif()

5. 额外建议

  • 编译器检查:这些标志主要是 GCC/Clang 的。如果项目需要支持 MSVC (Windows Visual Studio),这些标志会导致错误。
    • 改进:建议加上编译器检查:
      if(CMAKE_BUILD_TYPE STREQUAL "Release" AND (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang"))
          # ... 设置标志 ...
      endif()
  • PIE (Position Independent Executable):现代 Linux 系统通常要求开启 PIE (ASLR 的基础)。
    • 建议:CMake 较新版本默认会开启 PIE。如果不确定,可以检查 CMAKE_POSITION_INDEPENDENT_CODE 或在链接标志中加入 -pie(通常 -fPIE 是默认的)。目前的 diff 中没有显式添加 PIE,但在现代 CMake 中通常是默认开启的。如果是老版本 CMake,建议显式检查。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, wangrong1069

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wangrong1069
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit ff94644 into linuxdeepin:master Feb 2, 2026
21 checks passed
@wangrong1069 wangrong1069 deleted the pr0131 branch February 2, 2026 02:27
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 this pull request may close these issues.

3 participants