lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <00c8d676-0389-47aa-b50a-66a9aa737ff8@intel.com> Date: Mon, 23 Oct 2023 17:29:48 -0700 From: Jacob Keller <jacob.e.keller@...el.com> To: Christophe JAILLET <christophe.jaillet@...adoo.fr>, <dchickles@...vell.com>, <sburla@...vell.com>, <fmanlunas@...vell.com>, <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>, <veerasenareddy.burru@...ium.com> CC: <felix.manlunas@...ium.com>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <kernel-janitors@...r.kernel.org> Subject: Re: [PATCH net 2/2] liquidio: Simplify octeon_download_firmware() On 10/22/2023 1:59 PM, Christophe JAILLET wrote: > In order to remove the usage of strncat(), write directly at the rigth > place in the 'h->bootcmd' array and check if the output is truncated. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr> > --- > The goal is to potentially remove the strncat() function from the kernel. > Their are only few users and most of them use it wrongly. > > This patch is compile tested only. > --- > .../net/ethernet/cavium/liquidio/octeon_console.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_console.c b/drivers/net/ethernet/cavium/liquidio/octeon_console.c > index bd6baf2872a5..f1f0d7a0309a 100644 > --- a/drivers/net/ethernet/cavium/liquidio/octeon_console.c > +++ b/drivers/net/ethernet/cavium/liquidio/octeon_console.c > @@ -802,19 +802,17 @@ static int octeon_console_read(struct octeon_device *oct, u32 console_num, > } > > #define FBUF_SIZE (4 * 1024 * 1024) > -#define MAX_BOOTTIME_SIZE 80 > > int octeon_download_firmware(struct octeon_device *oct, const u8 *data, > size_t size) > { > struct octeon_firmware_file_header *h; > - char boottime[MAX_BOOTTIME_SIZE]; > struct timespec64 ts; > u32 crc32_result; > + u32 i, rem, used; > u64 load_addr; > u32 image_len; > int ret = 0; > - u32 i, rem; > > if (size < sizeof(struct octeon_firmware_file_header)) { > dev_err(&oct->pci_dev->dev, "Firmware file too small (%d < %d).\n", > @@ -896,16 +894,15 @@ int octeon_download_firmware(struct octeon_device *oct, const u8 *data, > * Octeon always uses UTC time. so timezone information is not sent. > */ > ktime_get_real_ts64(&ts); > - ret = snprintf(boottime, MAX_BOOTTIME_SIZE, > + > + used = strnlen(h->bootcmd, sizeof(h->bootcmd)); > + ret = snprintf(h->bootcmd + used, sizeof(h->bootcmd) - used, > " time_sec=%lld time_nsec=%ld", > (s64)ts.tv_sec, ts.tv_nsec); Maybe I am missing something here, but why not combine the boottime with the original write of bootcmd? I guess this is being updated periodically? The overall solution used here feels like the wrong way to do it... I guess the bootcmd is setup else where but I couldn't find the relevant code for that. What you did seems ok to me, but it still feels like there is a simpler way to achieve the desired result. Thanks, Jake
Powered by blists - more mailing lists