Linux clock framework development
 help / color / mirror / Atom feed
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
To: "seanjc@google.com" <seanjc@google.com>,
	"lyican53@gmail.com" <lyican53@gmail.com>
Cc: "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	Xiubo Li <xiubli@redhat.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"sboyd@kernel.org" <sboyd@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"idryomov@gmail.com" <idryomov@gmail.com>,
	"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"mturquette@baylibre.com" <mturquette@baylibre.com>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>
Subject: RE: [RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0
Date: Wed, 17 Sep 2025 17:34:29 +0000	[thread overview]
Message-ID: <1c6b3cd74e303fa8ab8b4853986fd4cb8c7c8541.camel@ibm.com> (raw)
In-Reply-To: <FF69D584-EEF9-4B5A-BE30-24EEBF354780@gmail.com>

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

      reply	other threads:[~2025-09-17 17:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-15  2:51 [RFC] Fix potential undefined behavior in __builtin_clz usage with GCC 11.1.0 陈华昭
2025-09-15 14:02 ` Sean Christopherson
2025-09-15 18:46 ` Viacheslav Dubeyko
2025-09-17 10:04   ` 陈华昭(Lyican)
2025-09-17 17:34     ` Viacheslav Dubeyko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1c6b3cd74e303fa8ab8b4853986fd4cb8c7c8541.camel@ibm.com \
    --to=slava.dubeyko@ibm.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=jejb@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=lyican53@gmail.com \
    --cc=martin.petersen@oracle.com \
    --cc=mturquette@baylibre.com \
    --cc=pbonzini@redhat.com \
    --cc=sboyd@kernel.org \
    --cc=seanjc@google.com \
    --cc=xiubli@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox