Skip to content

chore: Update compiler flags for security enhancements#383

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wangrong1069:pr0130
Jan 31, 2026
Merged

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

Conversation

@wangrong1069
Copy link
Contributor

@wangrong1069 wangrong1069 commented Jan 30, 2026

As title

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

Summary by Sourcery

Harden the release build configuration and adjust installation settings for plugins and runtime paths.

Build:

  • Enable additional compiler and linker hardening flags for release builds, including fortification, stack protections, and secure format handling.
  • Turn on verbose Makefile output for release builds to aid in inspecting applied build flags.
  • Remove custom RPATH and plugin installation targets from the src CMake configuration to rely on default or external runtime and plugin path handling.

As title

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

sourcery-ai bot commented Jan 30, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds hardened compiler and linker flags for Release builds and removes some plugin-related install and RPATH settings from the src CMake configuration.

Flow diagram for CMake build type handling and plugin install removal

flowchart TD
    Start[CMake configuration start]
    CheckType{Build type}

    Start --> CheckType

    CheckType -->|Release| EnableHardening["Set HARDENING_FLAGS and append to C and CXX flags"]
    EnableHardening --> SetLinkerFlags["Append -Wl,-z,relro and -Wl,-z,now to executable linker flags"]
    SetLinkerFlags --> Continue[Continue CMake configuration]

    CheckType -->|Not Release| Continue

    Continue --> SrcCMake[src/CMakeLists.txt]

    subgraph PreviousBehavior
        RPathSetting["Set target property INSTALL_RPATH for BIN_NAME"]
        PluginInstall1["Install LIB_NAME to DTK_QML_APP_PLUGIN_PATH"]
        PluginInstall2["Install LIB_NAME to PREFIX/plugins/imageformats"]
    end

    SrcCMake -->|Before PR| RPathSetting
    RPathSetting --> PluginInstall1
    PluginInstall1 --> PluginInstall2

    SrcCMake -->|After PR| End[Configuration without RPATH and plugin target installs]
Loading

File-Level Changes

Change Details Files
Enable security-oriented compiler and linker flags for Release builds.
  • Wrap new hardening configuration in a CMAKE_BUILD_TYPE STREQUAL "Release" condition to avoid affecting non-Release builds.
  • Enable verbose makefile output in Release builds to make applied flags visible during compilation.
  • Define a HARDENING_FLAGS variable with fortification, stack protection, format security warnings-as-errors, and related options, and append it to both CMAKE_C_FLAGS and CMAKE_CXX_FLAGS.
  • Append RELRO and immediate binding linker flags to CMAKE_EXE_LINKER_FLAGS for hardened executables.
CMakeLists.txt
Remove DTK QML plugin RPATH and library installation entries from the src CMake configuration.
  • Remove setting of INSTALL_RPATH on the main binary, previously pointing to DTK QML plugin path.
  • Remove installation of the LIB_NAME target into the DTK QML application plugin path and imageformats plugin directory, leaving only desktop, icon, service, and translation installs.
src/CMakeLists.txt

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

Hey - I've left some high level feedback:

  • Consider using generator expressions or CMAKE__FLAGS_RELEASE / CMAKE_EXE_LINKER_FLAGS_RELEASE instead of checking CMAKE_BUILD_TYPE == "Release", so the hardening also works correctly with multi-config generators (e.g. Ninja Multi-Config, Visual Studio).
  • Appending a hard-coded set of flags (including -g and -O2) to CMAKE_CXX_FLAGS/CMAKE_C_FLAGS may override or conflict with existing toolchain or distro flags; it may be safer to limit this to the hardening-specific options only and/or use add_compile_options/add_link_options so the defaults remain intact.
  • Enabling CMAKE_VERBOSE_MAKEFILE for Release builds may significantly increase log noise in normal build environments; you might want to keep this off by default and only enable it conditionally (e.g., via a cache option or for Debug builds).
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider using generator expressions or CMAKE_<LANG>_FLAGS_RELEASE / CMAKE_EXE_LINKER_FLAGS_RELEASE instead of checking CMAKE_BUILD_TYPE == "Release", so the hardening also works correctly with multi-config generators (e.g. Ninja Multi-Config, Visual Studio).
- Appending a hard-coded set of flags (including -g and -O2) to CMAKE_CXX_FLAGS/CMAKE_C_FLAGS may override or conflict with existing toolchain or distro flags; it may be safer to limit this to the hardening-specific options only and/or use add_compile_options/add_link_options so the defaults remain intact.
- Enabling CMAKE_VERBOSE_MAKEFILE for Release builds may significantly increase log noise in normal build environments; you might want to keep this off by default and only enable it conditionally (e.g., via a cache option or for Debug builds).

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码 diff 主要是对构建配置文件 CMakeLists.txt 的修改,主要涉及两个方面:在 Release 模式下增加了编译/链接的安全加固选项,以及移除了一些关于安装路径和库文件的配置。

以下是对这段 diff 的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个维度:

1. 语法逻辑

  • CMAKE_BUILD_TYPE 的判断
    • 代码使用了 if(CMAKE_BUILD_TYPE STREQUAL "Release")。这在单配置生成器(如 Unix Makefiles, Ninja)下是有效的。
    • 潜在问题:在多配置生成器(如 Visual Studio, Xcode)中,CMAKE_BUILD_TYPE 变量通常是空的,因为构建类型是在生成时(IDE中选择)确定的。虽然从文件名和上下文(DTK, deepin-album)看这是一个 Linux 项目,主要使用单配置生成器,但为了代码的健壮性,建议使用 CMake 的生成器表达式。
    • 改进建议:如果项目仅针对 Linux/Unix,当前写法可以接受。如果希望更通用,可以使用 if(CMAKE_BUILD_TYPE STREQUAL "Release" OR NOT CMAKE_BUILD_TYPE)(后者处理多配置情况),或者直接使用生成器表达式(如 $<CONFIG:Release>)来添加标志,但这在 set 命令中处理起来比较复杂。鉴于这是 Linux 项目,当前逻辑基本正确。

2. 代码质量

  • CMAKE_VERBOSE_MAKEFILE
    • 代码在 Release 模式下设置了 set(CMAKE_VERBOSE_MAKEFILE ON)
    • 意见:这通常用于调试构建过程。在 Release 模式下开启它会输出大量编译信息,可能会污染日志。除非是为了验证 Release 模式下的编译参数是否正确,否则不建议默认开启,或者建议将其移到外部构建脚本中。
  • 字符串拼接与变量定义
    • set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${HARDENING_FLAGS}") 这种写法是标准的,但要注意如果 CMAKE_CXX_FLAGS 中已有某些标志,HARDENING_FLAGS 追加在后面是正确的(后面的标志通常会覆盖前面的)。
  • 移除的代码
    • install(TARGETS ${LIB_NAME} ...) 被移除了。这意味着 ${LIB_NAME} 这个库将不再被安装到系统中。
    • 审查意见:这是一个重大的功能性变更。如果 ${LIB_NAME} 是一个插件(如 imageformats),移除安装会导致应用在运行时找不到该插件,从而功能缺失(例如无法加载某种图片格式)。请确认这是有意为之(例如该库已被静态链接到主程序,或者不再需要),否则会导致运行时错误。

3. 代码性能

  • 优化等级
    • HARDENING_FLAGS 中包含了 -O2
    • 意见:这是一个合理的性能优化级别。虽然 -O3 可能会优化更多,但 -O2 通常在体积和速度之间有更好的平衡,且安全性更好(某些激进优化可能导致未定义行为暴露)。
  • Debug 信息
    • HARDENING_FLAGS 中包含了 -g
    • 意见:在 Release 模式下包含 -g 会生成调试符号,这会增加二进制文件的大小。这对于生产环境发布包来说可能不是最优的(通常用户不需要调试符号)。但如果是为了能够分析 Release 版本的崩溃堆栈,这是必要的。建议权衡文件大小和调试需求。

4. 代码安全

这是本次修改的重点,添加的加固选项非常全面且专业。

  • 编译标志 (HARDENING_FLAGS)
    • -D_FORTIFY_SOURCE=2:良好的实践,在运行时检查缓冲区溢出等常见漏洞。
    • -fstack-protector-strong:启用强堆栈保护,防止栈溢出攻击。
    • -fstack-clash-protection:防止栈冲突攻击。
    • -Wformat -Werror=format-security:强制检查格式化字符串漏洞,并将警告视为错误,这是非常必要的安全措施。
    • -Wdate-time:检测嵌入二进制文件中的时间戳,有助于构建的可复现性。
    • -ffile-prefix-map=...:用于移除构建路径,有助于构建的可复现性和安全性(不暴露开发者机器路径)。
  • 链接标志
    • -Wl,-z,relro -Wl,-z,now:启用 RELRO (Relocation Read-Only) 和 BIND NOW,保护 GOT/PLT 表,防止重定位攻击。
  • 安全性总结:这些标志组合符合现代 Linux 安全编译标准(如 Debian 的 hardened flags),能显著提高二进制文件的抗攻击能力。

综合改进建议

  1. 关于 CMAKE_VERBOSE_MAKEFILE
    建议移除 set(CMAKE_VERBOSE_MAKEFILE ON),除非确实需要调试 Release 构建的命令行。这通常是一个开发者偏好设置,不应硬编码在 CMakeLists.txt 中。

  2. 关于移除的 install 命令
    请务必再次确认移除 install(TARGETS ${LIB_NAME} ...) 的影响。如果 LIB_NAME 是动态库或插件,移除它会导致程序运行失败。如果是静态库且已被链接,则移除是正确的。

  3. 关于 -g (Debug Symbols)
    如果发布给最终用户的安装包对体积敏感,建议在打包阶段使用 strip 命令去除符号,或者将符号分离到单独的 dbgsym 包中。在 CMake 中保留 -g 是为了保留调试能力,这是一个好的做法,但需要配合后续的打包流程。

  4. 关于多配置生成器的兼容性(可选)
    虽然项目看起来是 Linux 专用,但为了代码规范,可以将 Release 标志的设置改为使用生成器表达式,或者添加注释说明仅适用于单配置生成器。例如:

    # 注意:以下配置主要针对单配置生成器(如 Unix Makefiles)
    if(CMAKE_BUILD_TYPE STREQUAL "Release")
       ...
    endif()
  5. 代码格式化
    HARDENING_FLAGS 的字符串非常长,建议使用括号或反斜杠进行换行,以提高可读性(虽然 CMake 对换行处理比较严格,但在长字符串中可以通过引号拼接实现)。

修正后的代码片段示例(针对建议1和5):

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

    # set(CMAKE_VERBOSE_MAKEFILE ON) # 建议移除或按需开启

    # 将长字符串拆分以提高可读性
    set(HARDENING_FLAGS
        "-Wdate-time"
        "-D_FORTIFY_SOURCE=2"
        "-g"
        "-O2"
        "-ffile-prefix-map=${CMAKE_SOURCE_DIR}=."
        "-fstack-protector-strong"
        "-fstack-clash-protection"
        "-Wformat"
        "-Werror=format-security"
    )
    
    # 使用 list join 或者直接拼接
    string(REPLACE ";" " " HARDENING_FLAGS "${HARDENING_FLAGS}")

    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()

@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

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 31, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 2b414de into linuxdeepin:master Jan 31, 2026
16 of 18 checks passed
@wangrong1069 wangrong1069 deleted the pr0130 branch January 31, 2026 05:10
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