| Message ID | 20231112110146.3879030-1-chentao@kylinos.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | i40e: Use correct buffer size | expand |
On Sun, Nov 12, 2023 at 07:01:46PM +0800, Kunwu Chan wrote: > The size of "i40e_dbg_command_buf" is 256, the size of "name" is > at most 256, plus a null character and the format size, > the total size should be 516. Hi Kunwu Chan, Thanks for your patch. I'm slightly confused as to why name is at most 256 bytes. I see that name is IFNAMSIZ = 16 bytes. In any case, perhaps we could make buf_size dependent on it's constituent variables, to make things a bit clearer and a bit more robust. Something like this (completely untested!): int buf_size = IFNAMSIZ + sizeof(i40e_dbg_command_buf) + 4; Also, I'm not clear if this addresses a problem that can manifest in practice. Which affects if it it should be treated as a fix for iwl-net with a fixes tag, or as a feature for iwl-next without a fixes tag. In either case, if you repost, please designate the target tree in the Subject line. Something like this: Subject: [PATCH iwl-next] ... Lastly, when reposting patches, please allow 24h to elapse since the previous posting. Link: https://docs.kernel.org/process/maintainer-netdev.html ...
Hi Simon,
Thank you very much for taking the valuable time to point out my
problems and shortcomings in detail.
It's my bad.I misinterpreted 'IFALIASZ' in 'include/uapi/linux/if.h' as
'IFNAMSIZ'. This led me to think that 'IFNAMSIZ' could be up to 256.
Sorry again for the trouble.
Yes, it is good code to dynamically calculate the size of the parts that
make up the 'buffer' and add them up to the size of the whole 'buffer',
I got lazy because I saw that the other parts had a lot of fixed 'buffer
size'. I will immediately modify the patch according to your detailed
suggestions.
From the code analysis, this place should have a 'snprintf truncation'
problem, but the impact may not be very big, I found the potential
problem during the compilation process, after changing the buffer size,
recompilation will not alarm.
I'll follow your detailed suggestions and remove the 'Fixes' tag and add
'iwl-next' to the subject.
Thank you again for your reply and guidance.
在 2023/11/13 17:31, Simon Horman 写道:
> [PATCH iwl-next]
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c index 999c9708def5..d42355fa7217 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c @@ -72,7 +72,7 @@ static ssize_t i40e_dbg_command_read(struct file *filp, char __user *buffer, { struct i40e_pf *pf = filp->private_data; int bytes_not_copied; - int buf_size = 256; + int buf_size = 516; char *buf; int len;
The size of "i40e_dbg_command_buf" is 256, the size of "name" is at most 256, plus a null character and the format size, the total size should be 516. Fixes: 02e9c290814c ("i40e: debugfs interface") Signed-off-by: Kunwu Chan <chentao@kylinos.cn> --- drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)