rust: cpufreq: Add CPUFREQ_TABLE_END as last table entry in TableBuilder::to_table

Dear Linux Kernel Maintainers,

The TableBuilder::to_table function adds Hertz(c_ulong::MAX).as_khz() as the last frequency entry in the frequency table.
But the C API expects the last entry to have frequency set to CPUFREQ_TABLE_END which is ~1u as per include/linux/cpufreq.h.

This prevents insertions of cpufreq modules that use the Rust API to build frequency tables.

Here is a PoC kernel module that uses the Rust API to build a frequency table and register a cpufreq driver. This module fails to load with the attached error message.

// SPDX-License-Identifier: GPL-2.0

//! Rust cpufreq proof-of-concept driver

use core::sync::atomic::{AtomicU32, Ordering};

  use kernel::{
      bindings,
      clk::Hertz,
      cpufreq,
      error::code::*,
      macros::vtable,
      prelude::*,
      sync::Arc,
  };

  const LOW_FREQ: Hertz = Hertz::from_mhz(1000);
  const HIGH_FREQ: Hertz = Hertz::from_mhz(2000);
  const BIOS_LIMIT_KHZ: u32 = 424_242;

  module! {
      type: CpuFreqPocModule,
      name: "cpufreq_poc",
      authors: ["Priya Govindasamy"],
      description: "PoC cpufreq driver for the TableBuilder::to_table bug",
      license: "GPL",
  }

  struct CpuFreqPocModule {
      _driver: cpufreq::Registration<CpuFreqPocDriver>,
  }

  struct CpuFreqPocPolicyData {
      table: cpufreq::TableBox,
      current_freq_khz: AtomicU32,
  }

#[derive(Default)]
struct CpuFreqPocDriver;

  #[vtable]
  impl cpufreq::Driver for CpuFreqPocDriver {
      const NAME: &'static CStr = c"cpufreq-poc";
      const FLAGS: u16 = 0;
      const BOOST_ENABLED: bool = false;

      type PData = Arc<CpuFreqPocPolicyData>;

      fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> {
          
          let mut builder = cpufreq::TableBuilder::new();
          builder.add(LOW_FREQ, 0, 0).unwrap();
          builder.add(HIGH_FREQ, 0, 1).unwrap();
          
          let table = builder.to_table().unwrap();
          let cpu = policy.cpu();

          policy.cpus().set(cpu);
          policy
              .set_min(LOW_FREQ)
              .set_max(HIGH_FREQ)
              .set_cpuinfo_min_freq(LOW_FREQ)
              .set_cpuinfo_max_freq(HIGH_FREQ)
              .set_suspend_freq(LOW_FREQ)
              .set_transition_latency_ns(cpufreq::DEFAULT_TRANSITION_LATENCY_NS);

          unsafe { policy.set_freq_table(&table) };

          Ok(Arc::new(
              CpuFreqPocPolicyData {
                  table,
                  current_freq_khz: AtomicU32::new(HIGH_FREQ.as_khz() as u32),
              },
              GFP_KERNEL,
          )?)
      }

      fn verify(data: &mut cpufreq::PolicyData) -> Result {
          data.generic_verify()
      }

      fn target_index(policy: &mut cpufreq::Policy, index: cpufreq::TableIndex) -> Result {
          let Some(data) = policy.data::<Self::PData>() else {
              return Err(ENOENT);
          };

          let table = &data.table;
          let freq = table.freq(index)?;
          let freq_khz = freq.as_khz() as u32;

          data.current_freq_khz.store(freq_khz, Ordering::Relaxed);
          Ok(())
      }

      fn get(policy: &mut cpufreq::Policy) -> Result<u32> {
          let Some(data) = policy.data::<Self::PData>() else {
              return Err(ENOENT);
          };

          let freq = data.current_freq_khz.load(Ordering::Relaxed);

          Ok(freq)
      }

      fn bios_limit(policy: &mut cpufreq::Policy, limit: &mut u32) -> Result {
          *limit = BIOS_LIMIT_KHZ;
          Ok(())
      }

      fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result {
          Ok(())
      }

}

  impl kernel::Module for CpuFreqPocModule {
      fn init(_module: &'static ThisModule) -> Result<Self> {
          let driver = match cpufreq::Registration::<CpuFreqPocDriver>::new() {
              Ok(driver) => driver,
              Err(err) => {
                  pr_err!(
                      "cpufreq_poc: cpufreq driver registration failed: errno={}\n",
                      err.to_errno()
                  );
                  return Err(err);
              }
          };

Ok(Self { _driver: driver })
}
}

  impl Drop for CpuFreqPocModule {
      fn drop(&mut self) {
          pr_info!("cpufreq_poc: unloaded\n");
      }
  }

Output:
==================================================================

[ 1166.691889] BUG: KASAN: slab-out-of-bounds in cpufreq_frequency_table_cpuinfo+0x141/0x2c0
[ 1166.692782] Read of size 4 at addr ffff888281ddc038 by task insmod/5462

[ 1166.693721] CPU: 13 UID: 0 PID: 5462 Comm: insmod Tainted: G           OE       7.2.0-rc1+ #27 PREEMPT(lazy) 
[ 1166.693730] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
[ 1166.693732] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
[ 1166.693735] Call Trace:
[ 1166.693740]  <TASK>
[ 1166.693749]  dump_stack_lvl+0x97/0xe0
[ 1166.693773]  print_report+0x175/0x6f0
[ 1166.693794]  ? __virt_addr_valid+0x33e/0x3a0
[ 1166.693810]  ? cpufreq_frequency_table_cpuinfo+0x141/0x2c0
[ 1166.693814]  ? kasan_complete_mode_report_info+0x45/0x230
[ 1166.693825]  ? cpufreq_frequency_table_cpuinfo+0x141/0x2c0
[ 1166.693829]  kasan_report+0xe4/0x120
[ 1166.693832]  ? cpufreq_frequency_table_cpuinfo+0x141/0x2c0
[ 1166.693837]  ? rust_helper_krealloc_node_align+0xd/0x20
[ 1166.693861]  __asan_report_load4_noabort+0x18/0x20
[ 1166.693865]  cpufreq_frequency_table_cpuinfo+0x141/0x2c0
[ 1166.693869]  ? __pfx__RNvXs_NtCsbuTvttuFvbr_6kernel3fmtINtB4_7AdapterRmENtNtCsjYlAz7NZ3Sw_4core3fmt7Display3fmtCsgx7gD2tBKZx_12to_table_poc+0x10/0x10 [to_table_poc]
[ 1166.693879]  cpufreq_table_validate_and_sort+0x56/0x390
[ 1166.693883]  ? _RNvMsf_NtCsbuTvttuFvbr_6kernel7cpufreqINtB5_12RegistrationNtCsgx7gD2tBKZx_12to_table_poc16CpuFreqPocDriverE13init_callbackBW_+0x22/0x120 [to_table_poc]
[ 1166.693889]  cpufreq_online+0xb0f/0x22a0
[ 1166.693895]  cpufreq_add_dev+0x70/0x160
[ 1166.693900]  subsys_interface_register+0x273/0x360
[ 1166.693923]  cpufreq_register_driver+0x387/0x580
[ 1166.693933]  _RNvXCsgx7gD2tBKZx_12to_table_pocNtB2_16CpuFreqPocModuleNtCsbuTvttuFvbr_6kernel6Module4init+0x98/0x1d0 [to_table_poc]
[ 1166.693938]  ? stack_depot_save_flags+0x185/0x6e0
[ 1166.693953]  ? __pfx__RNvXs_NtCsbuTvttuFvbr_6kernel3fmtINtB4_7AdapterRmENtNtCsjYlAz7NZ3Sw_4core3fmt7Display3fmtCsgx7gD2tBKZx_12to_table_poc+0x10/0x10 [to_table_poc]
[ 1166.693961]  init_module+0x15/0xff0 [to_table_poc]
[ 1166.693966]  do_one_initcall+0x118/0x4e0
[ 1166.693973]  ? do_init_module+0x63/0x740
[ 1166.694026]  ? __pfx_init_module+0x10/0x10 [to_table_poc]
[ 1166.694030]  ? __se_sys_finit_module+0x39d/0x5f0
[ 1166.694033]  ? __x64_sys_finit_module+0x7f/0x90
[ 1166.694036]  ? x64_sys_call+0xa72/0x3030
[ 1166.694040]  ? do_syscall_64+0x152/0x460
[ 1166.694064]  ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 1166.694073]  ? kvm_sched_clock_read+0x15/0x40
[ 1166.694082]  ? __lock_acquire+0xd2f/0xe80
[ 1166.694096]  ? kvm_sched_clock_read+0x15/0x40
[ 1166.694101]  ? debug_smp_processor_id+0x1b/0x30
[ 1166.694106]  ? _raw_spin_unlock_irqrestore+0x38/0x90
[ 1166.694114]  ? __this_cpu_preempt_check+0x17/0x20
[ 1166.694118]  ? lockdep_hardirqs_on+0xdb/0x160
[ 1166.694123]  ? debug_smp_processor_id+0x1b/0x30
[ 1166.694128]  ? kasan_save_alloc_info+0x3a/0x50
[ 1166.694132]  ? __kasan_kmalloc+0x94/0xb0
[ 1166.694138]  ? __kmalloc_cache_noprof+0x366/0x720
[ 1166.694151]  ? do_init_module+0x63/0x740
[ 1166.694154]  ? __kmalloc_cache_noprof+0x160/0x720
[ 1166.694158]  ? kasan_unpoison+0x4c/0x70
[ 1166.694162]  ? kasan_poison+0x44/0x60
[ 1166.694168]  do_init_module+0x23f/0x740
[ 1166.694176]  load_module+0x323c/0x3eb0
[ 1166.694188]  __se_sys_finit_module+0x39d/0x5f0
[ 1166.694202]  __x64_sys_finit_module+0x7f/0x90
[ 1166.694206]  x64_sys_call+0xa72/0x3030
[ 1166.694227]  do_syscall_64+0x152/0x460
[ 1166.694232]  ? debug_smp_processor_id+0x1b/0x30
[ 1166.694238]  ? do_syscall_64+0x39/0x460
[ 1166.694242]  ? __this_cpu_preempt_check+0x17/0x20
[ 1166.694247]  ? do_syscall_64+0x106/0x460
[ 1166.694250]  ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 1166.694254]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 1166.694257] RIP: 0033:0x7f809171e90d
[ 1166.694272] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d f3 b4 0f 00 f7 d8 64 89 01 48
[ 1166.694278] RSP: 002b:00007ffd0c3840c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[ 1166.694290] RAX: ffffffffffffffda RBX: 000055b8afd917d0 RCX: 00007f809171e90d
[ 1166.694292] RDX: 0000000000000000 RSI: 000055b898d74cd2 RDI: 0000000000000003
[ 1166.694294] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[ 1166.694296] R10: 0000000000000003 R11: 0000000000000246 R12: 000055b898d74cd2
[ 1166.694298] R13: 000055b8afd91790 R14: 000055b898d73888 R15: 000055b8afd918e0
[ 1166.694306]  </TASK>

[ 1166.731860] Allocated by task 5462:
[ 1166.732274]  kasan_save_track+0x2f/0x70
[ 1166.732283]  kasan_save_alloc_info+0x3a/0x50
[ 1166.732287]  __kasan_kmalloc+0x94/0xb0
[ 1166.732292]  __kmalloc_node_track_caller_noprof+0x53f/0x830
[ 1166.732298]  krealloc_node_align_noprof+0x189/0x3b0
[ 1166.732302]  rust_helper_krealloc_node_align+0xd/0x20
[ 1166.732308]  _RNvMs6_NtCsbuTvttuFvbr_6kernel7cpufreqNtB5_12TableBuilder3add+0x11e/0x2f0
[ 1166.732326]  _RNvMs6_NtCsbuTvttuFvbr_6kernel7cpufreqNtB5_12TableBuilder8to_table+0x3b/0x360
[ 1166.732332]  _RNvXs2_Csgx7gD2tBKZx_12to_table_pocNtB5_16CpuFreqPocDriverNtNtCsbuTvttuFvbr_6kernel7cpufreq6Driver4init+0x9a/0x630 [to_table_poc]
[ 1166.732340]  _RNvMsf_NtCsbuTvttuFvbr_6kernel7cpufreqINtB5_12RegistrationNtCsgx7gD2tBKZx_12to_table_poc16CpuFreqPocDriverE13init_callbackBW_+0x22/0x120 [to_table_poc]
[ 1166.732347]  cpufreq_online+0x920/0x22a0
[ 1166.732351]  cpufreq_add_dev+0x70/0x160
[ 1166.732355]  subsys_interface_register+0x273/0x360
[ 1166.732360]  cpufreq_register_driver+0x387/0x580
[ 1166.732365]  _RNvXCsgx7gD2tBKZx_12to_table_pocNtB2_16CpuFreqPocModuleNtCsbuTvttuFvbr_6kernel6Module4init+0x98/0x1d0 [to_table_poc]
[ 1166.732371]  init_module+0x15/0xff0 [to_table_poc]
[ 1166.732377]  do_one_initcall+0x118/0x4e0
[ 1166.732381]  do_init_module+0x23f/0x740
[ 1166.732386]  load_module+0x323c/0x3eb0
[ 1166.732390]  __se_sys_finit_module+0x39d/0x5f0
[ 1166.732394]  __x64_sys_finit_module+0x7f/0x90
[ 1166.732398]  x64_sys_call+0xa72/0x3030
[ 1166.732403]  do_syscall_64+0x152/0x460
[ 1166.732408]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

[ 1166.732614] The buggy address belongs to the object at ffff888281ddc000
                which belongs to the cache kmalloc-64 of size 64
[ 1166.733704] The buggy address is located 8 bytes to the right of
                allocated 48-byte region [ffff888281ddc000, ffff888281ddc030)

[ 1166.734993] The buggy address belongs to the physical page:
[ 1166.735584] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff888281ddce00 pfn:0x281ddc
[ 1166.735595] flags: 0x17ffffc0000200(workingset|node=0|zone=2|lastcpupid=0x1fffff)
[ 1166.735606] page_type: f5(slab)
[ 1166.735614] raw: 0017ffffc0000200 ffff8881000428c0 ffff888100040590 ffff888100040590
[ 1166.735618] raw: ffff888281ddce00 000000080020001c 00000000f5000000 0000000000000000
[ 1166.735620] page dumped because: kasan: bad access detected

[ 1166.735783] Memory state around the buggy address:
[ 1166.736299]  ffff888281ddbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1166.737006]  ffff888281ddbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1166.737654] >ffff888281ddc000: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
[ 1166.738309]                                         ^
[ 1166.738769]  ffff888281ddc080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 1166.739582]  ffff888281ddc100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 1166.740331] ==================================================================

Priya Bala Govindasamy (1):
rust: cpufreq: Add CPUFREQ_TABLE_END as last table entry in
TableBuilder::to_table

 rust/kernel/cpufreq.rs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

2.34.1

The TableBuilder::to_table function adds Hertz(c_ulong::MAX).as_khz() as the last frequency entry in the frequency table.
But the C API expects the last entry to have frequency set to CPUFREQ_TABLE_END which is ~1u as per include/linux/cpufreq.h.

Fix this by setting the last frequency entry to CPUFREQ_TABLE_END instead of Hertz(c_ulong::MAX).as_khz().

---

 rust/kernel/cpufreq.rs | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

index 58ac04c650a1..97ab97724931 100644
@@ -377,8 +377,14 @@ pub fn add(&mut self, freq: Hertz, flags: u32, driver_data: u32) -> Result {
     /// Consumes the [`TableBuilder`] and returns [`TableBox`].
     pub fn to_table(mut self) -> Result<TableBox> {
         // Add last entry to the table.
-        self.add(Hertz(c_ulong::MAX), 0, 0)?;
-
+        self.entries.push(
+            bindings::cpufreq_frequency_table {
+                flags: 0,
+                driver_data: 0,
+                frequency: bindings::CPUFREQ_TABLE_END as u32,
+            },
+            GFP_KERNEL,
+        )?;
         TableBox::new(self.entries)
     }
 }

2.34.1

Did you hand edit the patch ? The above line (---) shouldn't be here. Please fix the commit properly and don't edit the patch once created.

    fn push(&mut self, frequency: u32, flags: u32, driver_data: u32) -> Result {
        Ok(self.entries.push(
            bindings::cpufreq_frequency_table {
                frequency,
                flags,
                driver_data,
            },
            GFP_KERNEL,
        )?)
    }

    /// Adds a new entry to the table.
    pub fn add(&mut self, freq: Hertz, flags: u32, driver_data: u32) -> Result {
        self.push(freq.as_khz() as u32, flags, driver_data)
    }

viresh