Skip to content

Commit da32871

Browse files
solbjorndavem330
authored andcommitted
net: qed: fix buffer overflow on ethtool -d
When generating debug dump, driver firstly collects all data in binary form, and then performs per-feature formatting to human-readable if it is supported. For ethtool -d, this is roughly incorrect for two reasons. First of all, drivers should always provide only original raw dumps to Ethtool without any changes. The second, and more critical, is that Ethtool's output buffer size is strictly determined by ethtool_ops::get_regs_len(), and all data *must* fit in it. The current version of driver always returns the size of raw data, but the size of the formatted buffer exceeds it in most cases. This leads to out-of-bound writes and memory corruption. Address both issues by adding an option to return original, non-formatted debug data, and using it for Ethtool case. v2: - Expand commit message to make it more clear; - No functional changes. Fixes: c965db4 ("qed: Add support for debug data collection") Signed-off-by: Alexander Lobakin <[email protected]> Signed-off-by: Igor Russkikh <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 5fc6266 commit da32871

File tree

2 files changed

+14
-1
lines changed

2 files changed

+14
-1
lines changed

drivers/net/ethernet/qlogic/qed/qed.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,8 @@ struct qed_dev {
876876
struct qed_dbg_feature dbg_features[DBG_FEATURE_NUM];
877877
u8 engine_for_debug;
878878
bool disable_ilt_dump;
879+
bool dbg_bin_dump;
880+
879881
DECLARE_HASHTABLE(connections, 10);
880882
const struct firmware *firmware;
881883

drivers/net/ethernet/qlogic/qed/qed_debug.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7506,6 +7506,12 @@ static enum dbg_status format_feature(struct qed_hwfn *p_hwfn,
75067506
if (p_hwfn->cdev->print_dbg_data)
75077507
qed_dbg_print_feature(text_buf, text_size_bytes);
75087508

7509+
/* Just return the original binary buffer if requested */
7510+
if (p_hwfn->cdev->dbg_bin_dump) {
7511+
vfree(text_buf);
7512+
return DBG_STATUS_OK;
7513+
}
7514+
75097515
/* Free the old dump_buf and point the dump_buf to the newly allocagted
75107516
* and formatted text buffer.
75117517
*/
@@ -7733,7 +7739,9 @@ int qed_dbg_mcp_trace_size(struct qed_dev *cdev)
77337739
#define REGDUMP_HEADER_SIZE_SHIFT 0
77347740
#define REGDUMP_HEADER_SIZE_MASK 0xffffff
77357741
#define REGDUMP_HEADER_FEATURE_SHIFT 24
7736-
#define REGDUMP_HEADER_FEATURE_MASK 0x3f
7742+
#define REGDUMP_HEADER_FEATURE_MASK 0x1f
7743+
#define REGDUMP_HEADER_BIN_DUMP_SHIFT 29
7744+
#define REGDUMP_HEADER_BIN_DUMP_MASK 0x1
77377745
#define REGDUMP_HEADER_OMIT_ENGINE_SHIFT 30
77387746
#define REGDUMP_HEADER_OMIT_ENGINE_MASK 0x1
77397747
#define REGDUMP_HEADER_ENGINE_SHIFT 31
@@ -7771,6 +7779,7 @@ static u32 qed_calc_regdump_header(struct qed_dev *cdev,
77717779
feature, feature_size);
77727780

77737781
SET_FIELD(res, REGDUMP_HEADER_FEATURE, feature);
7782+
SET_FIELD(res, REGDUMP_HEADER_BIN_DUMP, 1);
77747783
SET_FIELD(res, REGDUMP_HEADER_OMIT_ENGINE, omit_engine);
77757784
SET_FIELD(res, REGDUMP_HEADER_ENGINE, engine);
77767785

@@ -7794,6 +7803,7 @@ int qed_dbg_all_data(struct qed_dev *cdev, void *buffer)
77947803
omit_engine = 1;
77957804

77967805
mutex_lock(&qed_dbg_lock);
7806+
cdev->dbg_bin_dump = true;
77977807

77987808
org_engine = qed_get_debug_engine(cdev);
77997809
for (cur_engine = 0; cur_engine < cdev->num_hwfns; cur_engine++) {
@@ -7993,6 +8003,7 @@ int qed_dbg_all_data(struct qed_dev *cdev, void *buffer)
79938003
QED_NVM_IMAGE_MDUMP, "QED_NVM_IMAGE_MDUMP", rc);
79948004
}
79958005

8006+
cdev->dbg_bin_dump = false;
79968007
mutex_unlock(&qed_dbg_lock);
79978008

79988009
return 0;

0 commit comments

Comments
 (0)