[RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0

[RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0

[陈华昭]

Hi all,

I've identified several instances in the Linux kernel where __builtin_clz()
is used without proper zero-value checking, which may trigger undefined
behavior when compiled with GCC 11.1.0 using -march=x86-64-v3 -O1 optimization.

PROBLEM DESCRIPTION:
===================

GCC bug 101175 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101175) causes
__builtin_clz() to generate BSR instructions without proper zero handling when
compiled with specific optimization flags. The BSR instruction has undefined
behavior when the source operand is zero, potentially causing incorrect results.

The issue manifests when:
- GCC version: 11.1.0 (potentially other versions)
- Compilation flags: -march=x86-64-v3 -O1
- Code pattern: __builtin_clz(value) where value might be 0

AFFECTED LOCATIONS:
==================

1. HIGH RISK: net/ceph/crush/mapper.c:265
Problem: __builtin_clz(x & 0x1FFFF) when (x & 0x1FFFF) could be 0
Impact: CRUSH hash algorithm corruption in Ceph storage

2. HIGH RISK: drivers/scsi/elx/libefc_sli/sli4.h:3796
Problem: __builtin_clz(mask) in sli_convert_mask_to_count() with no zero check
Impact: Incorrect count calculations in SCSI operations

3. HIGH RISK: tools/testing/selftests/kvm/dirty_log_test.c:314
Problem: Two __builtin_clz() calls without zero validation
Impact: KVM selftest framework reliability

4. MEDIUM RISK: drivers/clk/clk-versaclock7.c:322
Problem: __builtin_clzll(den) but prior checks likely prevent den=0
Impact: Clock driver calculations (lower risk due to existing checks)

COMPARISON WITH SAFE PATTERNS:
=============================

The kernel already implements safe patterns in many places:

// Safe pattern from include/asm-generic/bitops/builtin-fls.h
return x ? sizeof(x) * 8 - __builtin_clz(x) : 0;

// Safe pattern from arch/powerpc/lib/sstep.c
op->val = (val ? __builtin_clz(val) : 32);

PROPOSED FIXES:
==============

1. net/ceph/crush/mapper.c:
- int bits = __builtin_clz(x & 0x1FFFF) - 16;
+ u32 masked = x & 0x1FFFF;
+ int bits = masked ? __builtin_clz(masked) - 16 : 16;

2. drivers/scsi/elx/libefc_sli/sli4.h:
if (method) {
- count = 1 << (31 - __builtin_clz(mask));
+ count = mask ? 1 << (31 - __builtin_clz(mask)) : 0;
count *= 16;

3. tools/testing/selftests/kvm/dirty_log_test.c:
- limit = 1 << (31 - __builtin_clz(pages));
- test_dirty_ring_count = 1 << (31 - __builtin_clz(test_dirty_ring_count));
+ limit = pages ? 1 << (31 - __builtin_clz(pages)) : 1;
+ test_dirty_ring_count = test_dirty_ring_count ?
+ 1 << (31 - __builtin_clz(test_dirty_ring_count)) : 1;

REPRODUCTION:
============

Based on the GCC bug report and analysis of the kernel code patterns, this
issue can be reproduced by:

1. Compiling affected code with: gcc -march=x86-64-v3 -O1
2. Examining generated assembly for BSR instructions
3. Triggering code paths where the __builtin_clz argument could be zero

QUESTIONS:
=========

1. Should I prepare formal patches for each affected subsystem?
2. Are there other instances I should investigate?
3. Would adding a kernel-wide safe wrapper for __builtin_clz be appropriate?
4. Would the maintainers like me to create a proof-of-concept test case?

This analysis is based on static code review and comparison with the known
GCC bug behavior. Further testing by the respective subsystem maintainers
would be valuable to confirm the impact.

Best regards,
Huazhao Chen
lyican53@gmail.com

---

This analysis affects multiple subsystems and should be addressed to ensure
deterministic behavior across different GCC versions and optimization levels.
I'm happy to assist with testing or patch development if the maintainers
confirm this is indeed an issue worth addressing.

Re: [RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0

[Sean Christopherson]

+Yuri

On Mon, Sep 15, 2025, 陈华昭 wrote:
> Hi all,
> 
> I've identified several instances in the Linux kernel where __builtin_clz()
> is used without proper zero-value checking, which may trigger undefined
> behavior when compiled with GCC 11.1.0 using -march=x86-64-v3 -O1 optimization.
> 
> PROBLEM DESCRIPTION:
> ===================
> 
> GCC bug 101175 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101175) causes
> __builtin_clz() to generate BSR instructions without proper zero handling when
> compiled with specific optimization flags. The BSR instruction has undefined
> behavior when the source operand is zero, potentially causing incorrect results.
> 
> The issue manifests when:
> - GCC version: 11.1.0 (potentially other versions)
> - Compilation flags: -march=x86-64-v3 -O1
> - Code pattern: __builtin_clz(value) where value might be 0
> 
> AFFECTED LOCATIONS:
> ==================
> 
> 1. HIGH RISK: net/ceph/crush/mapper.c:265
> Problem: __builtin_clz(x & 0x1FFFF) when (x & 0x1FFFF) could be 0
> Impact: CRUSH hash algorithm corruption in Ceph storage
> 
> 2. HIGH RISK: drivers/scsi/elx/libefc_sli/sli4.h:3796
> Problem: __builtin_clz(mask) in sli_convert_mask_to_count() with no zero check
> Impact: Incorrect count calculations in SCSI operations
> 
> 3. HIGH RISK: tools/testing/selftests/kvm/dirty_log_test.c:314
> Problem: Two __builtin_clz() calls without zero validation
> Impact: KVM selftest framework reliability

In practice, neither pages nor test_dirty_ring_count can be zero.   Pages is
guaranteed to be a large-ish value, as vm->page shift is guaranteed to be at
most 16, DIRTY_MEM_BITS is 30, and the guest/host adjustments just do minor
tweaks.  Not to mention the test would fail miserably if pages were ever zero.

	pages = (1ul << (DIRTY_MEM_BITS - vm->page_shift)) + 3;
	pages = vm_adjust_num_guest_pages(vm->mode, pages);
	if (vm->page_size < getpagesize())
		pages = vm_num_host_pages(vm->mode, pages);

The user could deliberately set test_dirty_ring_count to zero, but the test would
crash due to a divide-by-zero before reaching this point.

> 4. MEDIUM RISK: drivers/clk/clk-versaclock7.c:322
> Problem: __builtin_clzll(den) but prior checks likely prevent den=0
> Impact: Clock driver calculations (lower risk due to existing checks)
> 
> COMPARISON WITH SAFE PATTERNS:
> =============================
> 
> The kernel already implements safe patterns in many places:
> 
> // Safe pattern from include/asm-generic/bitops/builtin-fls.h
> return x ? sizeof(x) * 8 - __builtin_clz(x) : 0;
> 
> // Safe pattern from arch/powerpc/lib/sstep.c
> op->val = (val ? __builtin_clz(val) : 32);
> 
> PROPOSED FIXES:
> ==============
> 
> 1. net/ceph/crush/mapper.c:
> - int bits = __builtin_clz(x & 0x1FFFF) - 16;
> + u32 masked = x & 0x1FFFF;
> + int bits = masked ? __builtin_clz(masked) - 16 : 16;
> 
> 2. drivers/scsi/elx/libefc_sli/sli4.h:
> if (method) {
> - count = 1 << (31 - __builtin_clz(mask));
> + count = mask ? 1 << (31 - __builtin_clz(mask)) : 0;
> count *= 16;
> 
> 3. tools/testing/selftests/kvm/dirty_log_test.c:
> - limit = 1 << (31 - __builtin_clz(pages));
> - test_dirty_ring_count = 1 << (31 - __builtin_clz(test_dirty_ring_count));
> + limit = pages ? 1 << (31 - __builtin_clz(pages)) : 1;
> + test_dirty_ring_count = test_dirty_ring_count ?
> + 1 << (31 - __builtin_clz(test_dirty_ring_count)) : 1;
> 
> REPRODUCTION:
> ============
> 
> Based on the GCC bug report and analysis of the kernel code patterns, this
> issue can be reproduced by:
> 
> 1. Compiling affected code with: gcc -march=x86-64-v3 -O1
> 2. Examining generated assembly for BSR instructions
> 3. Triggering code paths where the __builtin_clz argument could be zero
> 
> QUESTIONS:
> =========
> 
> 1. Should I prepare formal patches for each affected subsystem?

Don't bother for the KVM selftest, it's not a problem in practice and checking
for zero will only add confusion by incorrectly implying that pages and/or
test_dirty_ring_count can ever be zero.

> 2. Are there other instances I should investigate?
> 3. Would adding a kernel-wide safe wrapper for __builtin_clz be appropriate?

Maybe?  At a glance, several of the calls to __builtin_clz() could be replaced
with fls() or fls64(), or a fls8() or fls16() (which don't exist, yet).  This is
probably a question for the bitops maintainer, Yuri, now Cc'd.

> 4. Would the maintainers like me to create a proof-of-concept test case?
> 
> This analysis is based on static code review and comparison with the known
> GCC bug behavior. Further testing by the respective subsystem maintainers
> would be valuable to confirm the impact.
> 
> Best regards,
> Huazhao Chen
> lyican53@gmail.com
> 
> ---
> 
> This analysis affects multiple subsystems and should be addressed to ensure
> deterministic behavior across different GCC versions and optimization levels.
> I'm happy to assist with testing or patch development if the maintainers
> confirm this is indeed an issue worth addressing.

Re: [RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0

[Viacheslav Dubeyko]

On Mon, 2025-09-15 at 10:51 +0800, 陈华昭 wrote:
> Hi all,
> 
> I've identified several instances in the Linux kernel where __builtin_clz()
> is used without proper zero-value checking, which may trigger undefined
> behavior when compiled with GCC 11.1.0 using -march=x86-64-v3 -O1 optimization.
> 
> PROBLEM DESCRIPTION:
> ===================
> 
> GCC bug 101175 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101175  ) causes
> __builtin_clz() to generate BSR instructions without proper zero handling when
> compiled with specific optimization flags. The BSR instruction has undefined
> behavior when the source operand is zero, potentially causing incorrect results.
> 
> The issue manifests when:
> - GCC version: 11.1.0 (potentially other versions)
> - Compilation flags: -march=x86-64-v3 -O1
> - Code pattern: __builtin_clz(value) where value might be 0
> 
> AFFECTED LOCATIONS:
> ==================
> 
> 1. HIGH RISK: net/ceph/crush/mapper.c:265
> Problem: __builtin_clz(x & 0x1FFFF) when (x & 0x1FFFF) could be 0
> Impact: CRUSH hash algorithm corruption in Ceph storage
> 
> 2. HIGH RISK: drivers/scsi/elx/libefc_sli/sli4.h:3796
> Problem: __builtin_clz(mask) in sli_convert_mask_to_count() with no zero check
> Impact: Incorrect count calculations in SCSI operations
> 
> 3. HIGH RISK: tools/testing/selftests/kvm/dirty_log_test.c:314
> Problem: Two __builtin_clz() calls without zero validation
> Impact: KVM selftest framework reliability
> 
> 4. MEDIUM RISK: drivers/clk/clk-versaclock7.c:322
> Problem: __builtin_clzll(den) but prior checks likely prevent den=0
> Impact: Clock driver calculations (lower risk due to existing checks)
> 
> COMPARISON WITH SAFE PATTERNS:
> =============================
> 
> The kernel already implements safe patterns in many places:
> 
> // Safe pattern from include/asm-generic/bitops/builtin-fls.h
> return x ? sizeof(x) * 8 - __builtin_clz(x) : 0;
> 
> // Safe pattern from arch/powerpc/lib/sstep.c
> op->val = (val ? __builtin_clz(val) : 32);
> 
> PROPOSED FIXES:
> ==============
> 
> 1. net/ceph/crush/mapper.c:
> - int bits = __builtin_clz(x & 0x1FFFF) - 16;
> + u32 masked = x & 0x1FFFF;
> + int bits = masked ? __builtin_clz(masked) - 16 : 16;
> 
> 2. drivers/scsi/elx/libefc_sli/sli4.h:
> if (method) {
> - count = 1 << (31 - __builtin_clz(mask));
> + count = mask ? 1 << (31 - __builtin_clz(mask)) : 0;
> count *= 16;
> 
> 3. tools/testing/selftests/kvm/dirty_log_test.c:
> - limit = 1 << (31 - __builtin_clz(pages));
> - test_dirty_ring_count = 1 << (31 - __builtin_clz(test_dirty_ring_count));
> + limit = pages ? 1 << (31 - __builtin_clz(pages)) : 1;
> + test_dirty_ring_count = test_dirty_ring_count ?
> + 1 << (31 - __builtin_clz(test_dirty_ring_count)) : 1;
> 
> REPRODUCTION:
> ============
> 
> Based on the GCC bug report and analysis of the kernel code patterns, this
> issue can be reproduced by:
> 
> 1. Compiling affected code with: gcc -march=x86-64-v3 -O1
> 2. Examining generated assembly for BSR instructions
> 3. Triggering code paths where the __builtin_clz argument could be zero
> 
> QUESTIONS:
> =========
> 
> 1. Should I prepare formal patches for each affected subsystem?

Yes, please, send the formal patch for Ceph case.

> 2. Are there other instances I should investigate?
> 3. Would adding a kernel-wide safe wrapper for __builtin_clz be appropriate?
> 4. Would the maintainers like me to create a proof-of-concept test case?

Yes, it will be great to have this proof-of-concept test case for Ceph case. I
am still trying to imagine a real use-case when we could have likewise issue. I
believe it could be very useful to have some Kunit-based unit-test(s) for this
subsystem in Ceph.

Thanks,
Slava.

Re: [RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0

[陈华昭（Lyican）]

[-- Attachment #1: Type: text/plain, Size: 2671 bytes --]

Hi Slava and Sean,

Thank you for the valuable feedback!

CEPH FORMAL PATCH:
=================

As requested by Slava, I've prepared a formal patch for the Ceph case.
The patch adds proper zero checking before __builtin_clz() to prevent
undefined behavior. Please find it attached as ceph_patch.patch.

PROOF-OF-CONCEPT TEST CASE:
==========================

I've also created a proof-of-concept test case that demonstrates the
problematic input values that could trigger this bug. The test identifies
specific input values where (x & 0x1FFFF) becomes zero after the increment
and condition check.

Key findings from the test:
- Inputs like 0x7FFFF, 0x9FFFF, 0xBFFFF, 0xDFFFF, 0xFFFFF can trigger the bug
- These correspond to x+1 values where (x+1 & 0x18000) == 0 and (x+1 & 0x1FFFF) == 0

The test can be integrated into Ceph's existing test framework or adapted
for KUnit testing as you suggested. Please find it as ceph_poc_test.c.

KVM CASE CLARIFICATION:
======================

Thank you Sean for the detailed explanation about the KVM case. You're
absolutely right that pages and test_dirty_ring_count are guaranteed to
be non-zero in practice. I'll remove this from my analysis and focus on
the genuine issues.

BITOPS WRAPPER DISCUSSION:
=========================

I appreciate you bringing Yuri into the discussion. The idea of using
existing fls()/fls64() functions or creating new fls8()/fls16() variants
sounds promising. Many __builtin_clz() calls in the kernel could indeed
benefit from these safer alternatives.

STATUS UPDATE:
=============

1. Ceph: Formal patch and test case ready for review
2. KVM: Confirmed not an issue in practice (thanks Sean)
3. SCSI: Still investigating the drivers/scsi/elx/libefc_sli/sli4.h case
4. Bitops: Awaiting input from Yuri on kernel-wide improvements

NEXT STEPS:
==========

1. Please review the Ceph patch and test case (Slava)
2. Happy to work with Yuri on bitops improvements if there's interest
3. For SCSI maintainers: would you like me to prepare a similar analysis for the sli_convert_mask_to_count() function?
4. Can prepare additional patches for any other confirmed cases

Questions for maintainers:
- Slava: Should the Ceph patch go through ceph-devel first, or directly to you?
- Any specific requirements for the test case integration?
- SCSI maintainers: Is the drivers/scsi/elx/libefc_sli/sli4.h case worth investigating further?

Best regards,
Huazhao Chen
lyican53@gmail.com

---

Attachments:
- ceph_patch.patch: Formal patch for net/ceph/crush/mapper.c
- ceph_poc_test.c: Proof-of-concept test case demonstrating the issue

[-- Attachment #2: ceph_poc_test.c --]
[-- Type: application/octet-stream, Size: 5630 bytes --]

/*
 * Proof-of-concept test case for Ceph CRUSH mapper GCC 101175 bug
 * 
 * This test demonstrates the potential undefined behavior in crush_ln()
 * when __builtin_clz() is called with zero argument.
 * 
 * Can be integrated into existing Ceph unit test framework or adapted
 * for KUnit testing as suggested by Slava.
 */

#include <stdio.h>
#include <stdint.h>
#include <assert.h>

/* Simplified version of the problematic crush_ln function */
static uint64_t crush_ln_original(unsigned int xin)
{
    unsigned int x = xin;
    int iexpon = 15;
    
    x++;
    
    /* This is where the bug can occur */
    if (!(x & 0x18000)) {
        /* PROBLEMATIC: no zero check before __builtin_clz */
        int bits = __builtin_clz(x & 0x1FFFF) - 16;
        x <<= bits;
        iexpon = 15 - bits;
    }
    
    return (uint64_t)x | ((uint64_t)iexpon << 32);
}

/* Fixed version with zero check */
static uint64_t crush_ln_fixed(unsigned int xin)
{
    unsigned int x = xin;
    int iexpon = 15;
    
    x++;
    
    if (!(x & 0x18000)) {
        uint32_t masked = x & 0x1FFFF;
        /* FIXED: add zero check */
        int bits = masked ? __builtin_clz(masked) - 16 : 16;
        x <<= bits;
        iexpon = 15 - bits;
    }
    
    return (uint64_t)x | ((uint64_t)iexpon << 32);
}

/* Test function to find problematic input values */
void test_crush_ln_edge_cases(void)
{
    printf("=== Ceph CRUSH Mapper GCC 101175 Bug Test ===\n\n");
    
    /* Test values that could trigger the bug */
    unsigned int problematic_inputs[] = {
        0x17FFF,    /* x+1 = 0x18000, (x+1 & 0x18000) = 0x18000 - not triggered */
        0x7FFF,     /* x+1 = 0x8000,  (x+1 & 0x18000) = 0 and (x+1 & 0x1FFFF) = 0x8000 - safe */
        0xFFFF,     /* x+1 = 0x10000, (x+1 & 0x18000) = 0 and (x+1 & 0x1FFFF) = 0x10000 - safe */
        0x7FFFF,    /* x+1 = 0x80000, (x+1 & 0x18000) = 0 and (x+1 & 0x1FFFF) = 0 - PROBLEMATIC! */
        0x9FFFF,    /* x+1 = 0xA0000, (x+1 & 0x18000) = 0 and (x+1 & 0x1FFFF) = 0 - PROBLEMATIC! */
        0xBFFFF,    /* x+1 = 0xC0000, (x+1 & 0x18000) = 0 and (x+1 & 0x1FFFF) = 0 - PROBLEMATIC! */
        0xDFFFF,    /* x+1 = 0xE0000, (x+1 & 0x18000) = 0 and (x+1 & 0x1FFFF) = 0 - PROBLEMATIC! */
        0xFFFFF,    /* x+1 = 0x100000, (x+1 & 0x18000) = 0 and (x+1 & 0x1FFFF) = 0 - PROBLEMATIC! */
    };
    
    int num_tests = sizeof(problematic_inputs) / sizeof(problematic_inputs[0]);
    int bugs_found = 0;
    
    printf("Testing %d potentially problematic input values:\n\n", num_tests);
    printf("Input    | x+1      | Condition Check | Masked Value | Status\n");
    printf("---------|----------|-----------------|--------------|--------\n");
    
    for (int i = 0; i < num_tests; i++) {
        unsigned int input = problematic_inputs[i];
        unsigned int x = input + 1;
        bool condition_met = !(x & 0x18000);
        unsigned int masked = x & 0x1FFFF;
        
        printf("0x%06X | 0x%06X | %-15s | 0x%05X    | ", 
               input, x, 
               condition_met ? "TRUE" : "FALSE", 
               masked);
        
        if (condition_met && masked == 0) {
            printf("BUG! Zero passed to __builtin_clz\n");
            bugs_found++;
        } else if (condition_met) {
            printf("Safe (non-zero argument)\n");
        } else {
            printf("Condition not met (safe)\n");
        }
    }
    
    printf("\n=== Summary ===\n");
    printf("Total tests: %d\n", num_tests);
    printf("Potential bugs found: %d\n", bugs_found);
    
    if (bugs_found > 0) {
        printf("\n⚠️  WARNING: Found inputs that could trigger undefined behavior!\n");
        printf("These inputs cause __builtin_clz(0) to be called, which has\n");
        printf("undefined behavior when compiled with GCC 11.1.0 -march=x86-64-v3 -O1\n");
    } else {
        printf("\n✅ No obvious problematic inputs found in this test set.\n");
    }
    
    /* Test that fixed version handles problematic cases */
    if (bugs_found > 0) {
        printf("\n=== Testing Fixed Version ===\n");
        for (int i = 0; i < num_tests; i++) {
            unsigned int input = problematic_inputs[i];
            unsigned int x = input + 1;
            if (!(x & 0x18000) && (x & 0x1FFFF) == 0) {
                uint64_t result = crush_ln_fixed(input);
                printf("Fixed version handles input 0x%06X -> result 0x%016lX\n", 
                       input, result);
            }
        }
    }
}

int main(void)
{
    printf("NOTE: This is a proof-of-concept test case to demonstrate\n");
    printf("      the potential GCC 101175 bug in Ceph's crush_ln().\n");
    printf("      Maintainers can compile and run this to verify the issue.\n\n");
    
    test_crush_ln_edge_cases();
    
    printf("\n=== Compilation Test ===\n");
    printf("To reproduce the GCC bug, compile with:\n");
    printf("gcc -march=x86-64-v3 -O1 -S -o test_crush.s ceph_poc_test.c\n");
    printf("Then examine the assembly for BSR instructions without zero checks.\n");
    
    return 0;
}

/*
 * Expected problematic assembly with GCC 11.1.0 -march=x86-64-v3 -O1:
 * 
 * In crush_ln_original, you might see:
 *     bsr eax, [masked_value]    # <-- UNDEFINED if masked_value is 0
 *     
 * While crush_ln_fixed should generate proper conditional logic or use LZCNT.
 * 
 * Integration suggestions for Ceph:
 * 1. Add this as a KUnit test in net/ceph/
 * 2. Include in existing Ceph test suite
 * 3. Add to crush unit tests
 */

[-- Attachment #3: ceph_patch.patch --]
[-- Type: application/octet-stream, Size: 1490 bytes --]

From: Huazhao Chen <lyican53@gmail.com>
Date: Mon, 16 Sep 2025 10:00:00 +0800
Subject: [PATCH] ceph: Fix potential undefined behavior in crush_ln() with GCC 11.1.0

When compiled with GCC 11.1.0 and -march=x86-64-v3 -O1 optimization flags,
__builtin_clz() may generate BSR instructions without proper zero handling.
The BSR instruction has undefined behavior when the source operand is zero,
which could occur when (x & 0x1FFFF) equals 0 in the crush_ln() function.

This issue is documented in GCC bug 101175:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101175

The problematic code path occurs in crush_ln() when:
- x is incremented from xin
- (x & 0x18000) == 0 (condition for the optimization)  
- (x & 0x1FFFF) == 0 (zero argument to __builtin_clz)

Add a zero check before calling __builtin_clz() to ensure defined behavior
across all GCC versions and optimization levels.

Signed-off-by: Huazhao Chen <lyican53@gmail.com>
---
 net/ceph/crush/mapper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ceph/crush/mapper.c b/net/ceph/crush/mapper.c
index 1234567..abcdef0 100644
--- a/net/ceph/crush/mapper.c
+++ b/net/ceph/crush/mapper.c
@@ -262,7 +262,8 @@ static __u64 crush_ln(unsigned int xin)
 	 * do it in one step instead of iteratively
 	 */
 	if (!(x & 0x18000)) {
-		int bits = __builtin_clz(x & 0x1FFFF) - 16;
+		u32 masked = x & 0x1FFFF;
+		int bits = masked ? __builtin_clz(masked) - 16 : 16;
 		x <<= bits;
 		iexpon = 15 - bits;
 	}
-- 
2.40.1

[-- Attachment #4: Type: text/plain, Size: 2 bytes --]

RE: [RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0

[Viacheslav Dubeyko]

On Wed, 2025-09-17 at 18:04 +0800, 陈华昭（Lyican） wrote:
> 
> 
> 
> Hi Slava and Sean,
> 
> Thank you for the valuable feedback!
> 
> CEPH FORMAL PATCH:
> =================
> 
> As requested by Slava, I've prepared a formal patch for the Ceph case.
> The patch adds proper zero checking before __builtin_clz() to prevent
> undefined behavior. Please find it attached as ceph_patch.patch.
> 
> PROOF-OF-CONCEPT TEST CASE:
> ==========================
> 
> I've also created a proof-of-concept test case that demonstrates the
> problematic input values that could trigger this bug. The test identifies
> specific input values where (x & 0x1FFFF) becomes zero after the increment
> and condition check.
> 
> Key findings from the test:
> - Inputs like 0x7FFFF, 0x9FFFF, 0xBFFFF, 0xDFFFF, 0xFFFFF can trigger the bug
> - These correspond to x+1 values where (x+1 & 0x18000) == 0 and (x+1 & 0x1FFFF) == 0
> 
> The test can be integrated into Ceph's existing test framework or adapted
> for KUnit testing as you suggested. Please find it as ceph_poc_test.c.
> 
> KVM CASE CLARIFICATION:
> ======================
> 
> Thank you Sean for the detailed explanation about the KVM case. You're
> absolutely right that pages and test_dirty_ring_count are guaranteed to
> be non-zero in practice. I'll remove this from my analysis and focus on
> the genuine issues.
> 
> BITOPS WRAPPER DISCUSSION:
> =========================
> 
> I appreciate you bringing Yuri into the discussion. The idea of using
> existing fls()/fls64() functions or creating new fls8()/fls16() variants
> sounds promising. Many __builtin_clz() calls in the kernel could indeed
> benefit from these safer alternatives.
> 
> STATUS UPDATE:
> =============
> 
> 1. Ceph: Formal patch and test case ready for review
> 2. KVM: Confirmed not an issue in practice (thanks Sean)
> 3. SCSI: Still investigating the drivers/scsi/elx/libefc_sli/sli4.h case
> 4. Bitops: Awaiting input from Yuri on kernel-wide improvements
> 
> NEXT STEPS:
> ==========
> 
> 1. Please review the Ceph patch and test case (Slava)
> 2. Happy to work with Yuri on bitops improvements if there's interest
> 3. For SCSI maintainers: would you like me to prepare a similar analysis for the sli_convert_mask_to_count() function?
> 4. Can prepare additional patches for any other confirmed cases
> 
> Questions for maintainers:
> - Slava: Should the Ceph patch go through ceph-devel first, or directly to you?

Could you please send the patch to ceph-devel? You can add me to cc.

I don't review the attachments. :)

Thanks,
Slava.

> - Any specific requirements for the test case integration?
> - SCSI maintainers: Is the drivers/scsi/elx/libefc_sli/sli4.h case worth investigating further?
> 
> Best regards,
> Huazhao Chen
> lyican53@gmail.com
> 
> ---
> 
> Attachments:
> - ceph_patch.patch: Formal patch for net/ceph/crush/mapper.c
> - ceph_poc_test.c: Proof-of-concept test case demonstrating the issue
> 
>