Skip to content

ocp-smart-extended-log: fix sizeof mismatch in malloc for get_c0_log_page()#3555

Open
sahmed-ibm wants to merge 1 commit into
linux-nvme:masterfrom
sahmed-ibm:fix-ocp-smart-extended-log-allocator-sizeof-op-mismatch
Open

ocp-smart-extended-log: fix sizeof mismatch in malloc for get_c0_log_page()#3555
sahmed-ibm wants to merge 1 commit into
linux-nvme:masterfrom
sahmed-ibm:fix-ocp-smart-extended-log-allocator-sizeof-op-mismatch

Conversation

@sahmed-ibm

Copy link
Copy Markdown
Contributor

The get_c0_log_page() function allocates a buffer @DaTa to hold the C0 SMART extended log page, but passes sizeof of an unrelated scalar type as the malloc operand instead of sizeof of the struct @DaTa points to.

If the compiler inserts alignment padding into the struct, the allocation will be smaller than the struct size, causing a heap buffer overflow when the struct fields are accessed beyond the allocated region.

Fix by using sizeof(*data) to ensure the correct amount of memory is allocated and zeroed for the struct.

…page()

The get_c0_log_page() function allocates a buffer @DaTa to hold
the C0 SMART extended log page, but passes sizeof of an unrelated
scalar type as the malloc operand instead of sizeof of the struct
@DaTa points to.

If the compiler inserts alignment padding into the struct, the
allocation will be smaller than the struct size, causing a heap
buffer overflow when the struct fields are accessed beyond the
allocated region.

Fix by using sizeof(*data) to ensure the correct amount of memory is
allocated and zeroed for the struct.

Signed-off-by: Sarah Ahmed <sarah.ahmed@ibm.com>
@igaw

igaw commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

according pahole

› pahole ./.build/nvme -C ocp_smart_extended_log
struct ocp_smart_extended_log {
	__u8                       physical_media_units_written[16]; /*     0    16 */
	__u8                       physical_media_units_read[16]; /*    16    16 */
	__u8                       bad_user_nand_blocks_raw[6]; /*    32     6 */
	__le16                     bad_user_nand_blocks_normalized; /*    38     2 */
	__u8                       bad_system_nand_blocks_raw[6]; /*    40     6 */
	__le16                     bad_system_nand_blocks_normalized; /*    46     2 */
	__le64                     xor_recovery_count;   /*    48     8 */
	__le64                     uncorrectable_read_err_count; /*    56     8 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	__le64                     soft_ecc_err_count;   /*    64     8 */
	__le32                     end_to_end_detected_err; /*    72     4 */
	__le32                     end_to_end_corrected_err; /*    76     4 */
	__u8                       system_data_used_percent; /*    80     1 */
	__u8                       refresh_counts[7];    /*    81     7 */
	__le32                     user_data_erase_count_max; /*    88     4 */
	__le32                     user_data_erase_count_min; /*    92     4 */
	__u8                       thermal_throttling_event_count; /*    96     1 */
	__u8                       thermal_throttling_current_status; /*    97     1 */
	__u8                       dssd_errata_version;  /*    98     1 */
	__u8                       dssd_point_version[2]; /*    99     2 */
	__u8                       dssd_minor_version[2]; /*   101     2 */
	__u8                       dssd_major_version;   /*   103     1 */
	__le64                     pcie_correctable_err_count; /*   104     8 */
	__le32                     incomplete_shoutdowns; /*   112     4 */
	__u8                       rsvd116[4];           /*   116     4 */
	__u8                       percent_free_blocks;  /*   120     1 */
	__u8                       rsvd121[7];           /*   121     7 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	__le16                     capacitor_health;     /*   128     2 */
	__u8                       nvme_base_errata_version; /*   130     1 */
	__u8                       nvme_cmdset_errata_version; /*   131     1 */
	__u8                       nvme_over_pcie_errate_version; /*   132     1 */
	__u8                       nvme_mi_errata_version; /*   133     1 */
	__u8                       rsvd134[2];           /*   134     2 */
	__le64                     unaligned_io;         /*   136     8 */
	__le64                     security_version;     /*   144     8 */
	__le64                     total_nuse;           /*   152     8 */
	__u8                       plp_start_count[16];  /*   160    16 */
	__u8                       endurance_estimate[16]; /*   176    16 */
	/* --- cacheline 3 boundary (192 bytes) --- */
	__le64                     pcie_link_retaining_count; /*   192     8 */
	__le64                     power_state_change_count; /*   200     8 */
	__le64                     lowest_permitted_fw_rev; /*   208     8 */
	__le16                     total_media_dies;     /*   216     2 */
	__le16                     total_die_failure_tolerance; /*   218     2 */
	__le16                     media_dies_offline;   /*   220     2 */
	__u8                       max_temperature_recorded; /*   222     1 */
	__u8                       form_factor;          /*   223     1 */
	__le64                     nand_avg_erase_count; /*   224     8 */
	__le32                     command_timeouts;     /*   232     4 */
	__le32                     sys_area_program_fail_count_raw; /*   236     4 */
	__u8                       sys_area_program_fail_count_normalized; /*   240     1 */
	__u8                       rsvd241[3];           /*   241     3 */
	__le32                     sys_area_uncorr_read_count_raw; /*   244     4 */
	__u8                       sys_area_uncorr_read_count_normalized; /*   248     1 */
	__u8                       rsvd249[3];           /*   249     3 */
	__le32                     sys_area_erase_fail_count_raw; /*   252     4 */
	/* --- cacheline 4 boundary (256 bytes) --- */
	__u8                       sys_area_erase_fail_count_normalized; /*   256     1 */
	__u8                       rsvd257[3];           /*   257     3 */
	__le16                     max_peak_power_capability; /*   260     2 */
	__le16                     current_max_avg_power; /*   262     2 */
	__u8                       lifetime_power_consumed[6]; /*   264     6 */
	__u8                       dssd_firmware_revision[8]; /*   270     8 */
	__u8                       dssd_firmware_build_uuid[16]; /*   278    16 */
	__u8                       dssd_firmware_build_label[64]; /*   294    64 */
	/* --- cacheline 5 boundary (320 bytes) was 38 bytes ago --- */
	__u8                       die_in_use_bad_nand_block_raw[6]; /*   358     6 */
	__le16                     die_in_use_bad_nand_block_normalized; /*   364     2 */
	__u8                       rsvd358[128];         /*   366   128 */
	/* --- cacheline 7 boundary (448 bytes) was 46 bytes ago --- */
	__le16                     log_page_version;     /*   494     2 */
	__u8                       log_page_guid[16];    /*   496    16 */

	/* size: 512, cachelines: 8, members: 67 */
};

which is in hex 0x200. This matches

 #define C0_SMART_CLOUD_ATTR_LEN                 0x200     

and later in the code we do

	nvme_init_get_log(&cmd, NVME_NSID_ALL,
			  (enum nvme_cmd_get_log_lid)OCP_LID_SMART,
			  NVME_CSI_NVM, data, C0_SMART_CLOUD_ATTR_LEN);

As far I understand your argument is that the compiler might insert padding? If this would happen the data struct would be useless.

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.

2 participants