| Message ID | 20231211025927.233449-1-chentao@kylinos.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | iavf: Fix null pointer dereference in iavf_print_link_message | expand |
On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote: > kasprintf() returns a pointer to dynamically allocated memory > which can be NULL upon failure. > > Fixes: 1978d3ead82c ("intel: fix string truncation warnings") No need for the allocation here, print to a buffer on the stack.
On 12/12/2023 1:28 PM, Jakub Kicinski wrote: > On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote: >> kasprintf() returns a pointer to dynamically allocated memory >> which can be NULL upon failure. >> >> Fixes: 1978d3ead82c ("intel: fix string truncation warnings") > > No need for the allocation here, print to a buffer on the stack. Sure, but I think that just takes us full circle back to where we started. reverting this to the previous code will add back W=1 warnings. The whole point of the commit mentioned above was to get a reasonable implementation that won't cause string truncation warnings. Is there some trick I don't know about to get an allocation which will not trigger snprintf and friends to print warnings from -Wformat-truncation > drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1425:60: warning: ‘%s’ directive output may be truncated writing 4 bytes into a region of size between 1 and 11 [-Wformat-truncation=] > drivers/net/ethernet/intel/iavf/iavf_virtchnl.c:1425:17: note: ‘snprintf’ output between 7 and 17 bytes into a destination of size 13 However the original warnings were for "%s" in strings. There should be a good way, but I don't know it so could use some help. -Jesse
On Tue, 12 Dec 2023 15:05:19 -0800 Jesse Brandeburg wrote: > On 12/12/2023 1:28 PM, Jakub Kicinski wrote: > > On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote: > >> kasprintf() returns a pointer to dynamically allocated memory > >> which can be NULL upon failure. > >> > >> Fixes: 1978d3ead82c ("intel: fix string truncation warnings") > > > > No need for the allocation here, print to a buffer on the stack. > > Sure, but I think that just takes us full circle back to where we > started. reverting this to the previous code will add back W=1 warnings. > > The whole point of the commit mentioned above was to get a reasonable > implementation that won't cause string truncation warnings. Is there > some trick I don't know about to get an allocation which will not > trigger snprintf and friends to print warnings from -Wformat-truncation Hm, it'd be nice if there was a flavor of snprintf which explicitly doesn't trigger this warning. Or perhaps a marking for the output buffer that says "truncation OK". Absent that, can we print to a buffer on the stack and copy? The link message is probably meh, but automation may get quite confused if a NIC suddenly stops reporting FW version..
Thanks for your reply. Sure, the only thing 'iavf_print_link_message' do is to print a msg by netdev_info. The 'iavf_virtchnl_completion' assume that no errors will be returned. Whether we could just execute 'netdev_info(netdev, "NIC Link is Up Speed is %s Full Duplex\n", speed? speed :"");' when 'speed' is null. Before commit '1978d3ead82c8', the buffer size is '#define IAVF_MAX_SPEED_STRLEN 13', whether we could use a bigger buffer size to avoid a null pointer. Such as '#define IAVF_MAX_SPEED_STRLEN 48'. On 2023/12/13 05:28, Jakub Kicinski wrote: > On Mon, 11 Dec 2023 10:59:27 +0800 Kunwu Chan wrote: >> kasprintf() returns a pointer to dynamically allocated memory >> which can be NULL upon failure. >> >> Fixes: 1978d3ead82c ("intel: fix string truncation warnings") > > No need for the allocation here, print to a buffer on the stack.
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c index 64c4443dbef9..1b50d351f28b 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c @@ -1444,6 +1444,8 @@ static void iavf_print_link_message(struct iavf_adapter *adapter) speed = kasprintf(GFP_KERNEL, "%d Mbps", link_speed_mbps); } + if (!speed) + return; netdev_info(netdev, "NIC Link is Up Speed is %s Full Duplex\n", speed); kfree(speed); }
kasprintf() returns a pointer to dynamically allocated memory which can be NULL upon failure. Fixes: 1978d3ead82c ("intel: fix string truncation warnings") Cc: Kunwu Chan <kunwu.chan@hotmail.com> Signed-off-by: Kunwu Chan <chentao@kylinos.cn> --- drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 2 ++ 1 file changed, 2 insertions(+)