| 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: <aKjgWZnfWShtvi8m@mini-arch>
Date: Fri, 22 Aug 2025 14:25:45 -0700
From: Stanislav Fomichev <stfomichev@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
almasrymina@...gle.com, sdf@...ichev.me, joe@...a.to,
linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next 1/4] selftests: drv-net: ncdevmem: remove use of
error()
On 08/22, Jakub Kicinski wrote:
> Using error() makes it impossible for callers to unwind their
> changes. Replace error() calls with proper error handling.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
Acked-by: Stanislav Fomichev <sdf@...ichev.me>
> ---
> .../selftests/drivers/net/hw/ncdevmem.c | 528 ++++++++++++------
> 1 file changed, 364 insertions(+), 164 deletions(-)
>
> diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
> index 71961a7688e6..e75adfed33ac 100644
> --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c
> +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
> @@ -115,6 +115,21 @@ struct memory_provider {
> size_t off, int n);
> };
>
> +static void pr_err(const char *fmt, ...)
> +{
> + va_list args;
> +
> + fprintf(stderr, "%s: ", TEST_PREFIX);
> +
> + va_start(args, fmt);
> + vfprintf(stderr, fmt, args);
> + va_end(args);
> +
> + if (errno != 0)
> + fprintf(stderr, ": %s", strerror(errno));
> + fprintf(stderr, "\n");
> +}
> +
> static struct memory_buffer *udmabuf_alloc(size_t size)
> {
> struct udmabuf_create create;
> @@ -123,27 +138,33 @@ static struct memory_buffer *udmabuf_alloc(size_t size)
>
> ctx = malloc(sizeof(*ctx));
> if (!ctx)
> - error(1, ENOMEM, "malloc failed");
> + return NULL;
>
> ctx->size = size;
>
> ctx->devfd = open("/dev/udmabuf", O_RDWR);
> - if (ctx->devfd < 0)
> - error(1, errno,
> - "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n",
> - TEST_PREFIX);
> + if (ctx->devfd < 0) {
> + pr_err("[skip,no-udmabuf: Unable to access DMA buffer device file]");
> + goto err_free_ctx;
> + }
>
> ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
> - if (ctx->memfd < 0)
> - error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX);
> + if (ctx->memfd < 0) {
> + pr_err("[skip,no-memfd]");
> + goto err_close_dev;
> + }
>
> ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK);
> - if (ret < 0)
> - error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
> + if (ret < 0) {
> + pr_err("[skip,fcntl-add-seals]");
> + goto err_close_memfd;
> + }
>
> ret = ftruncate(ctx->memfd, size);
> - if (ret == -1)
> - error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX);
> + if (ret == -1) {
> + pr_err("[FAIL,memfd-truncate]");
> + goto err_close_memfd;
> + }
>
> memset(&create, 0, sizeof(create));
>
> @@ -151,15 +172,29 @@ static struct memory_buffer *udmabuf_alloc(size_t size)
> create.offset = 0;
> create.size = size;
> ctx->fd = ioctl(ctx->devfd, UDMABUF_CREATE, &create);
> - if (ctx->fd < 0)
> - error(1, errno, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX);
> + if (ctx->fd < 0) {
> + pr_err("[FAIL, create udmabuf]");
> + goto err_close_fd;
> + }
>
> ctx->buf_mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED,
> ctx->fd, 0);
> - if (ctx->buf_mem == MAP_FAILED)
> - error(1, errno, "%s: [FAIL, map udmabuf]\n", TEST_PREFIX);
> + if (ctx->buf_mem == MAP_FAILED) {
> + pr_err("[FAIL, map udmabuf]");
> + goto err_close_fd;
> + }
>
> return ctx;
> +
> +err_close_fd:
> + close(ctx->fd);
> +err_close_memfd:
> + close(ctx->memfd);
> +err_close_dev:
> + close(ctx->devfd);
> +err_free_ctx:
> + free(ctx);
> + return NULL;
> }
>
> static void udmabuf_free(struct memory_buffer *ctx)
> @@ -217,7 +252,7 @@ static void print_nonzero_bytes(void *ptr, size_t size)
> putchar(p[i]);
> }
>
> -void validate_buffer(void *line, size_t size)
> +int validate_buffer(void *line, size_t size)
> {
> static unsigned char seed = 1;
> unsigned char *ptr = line;
> @@ -232,8 +267,10 @@ void validate_buffer(void *line, size_t size)
> "Failed validation: expected=%u, actual=%u, index=%lu\n",
> expected, ptr[i], i);
> errors++;
> - if (errors > 20)
> - error(1, 0, "validation failed.");
> + if (errors > 20) {
> + pr_err("validation failed");
> + return -1;
> + }
> }
> seed++;
> if (seed == do_validation)
> @@ -241,6 +278,7 @@ void validate_buffer(void *line, size_t size)
> }
>
> fprintf(stdout, "Validated buffer\n");
> + return 0;
> }
>
> static int rxq_num(int ifindex)
> @@ -279,7 +317,7 @@ static int rxq_num(int ifindex)
> system(command); \
> })
>
> -static int reset_flow_steering(void)
> +static void reset_flow_steering(void)
> {
> /* Depending on the NIC, toggling ntuple off and on might not
> * be allowed. Additionally, attempting to delete existing filters
> @@ -292,7 +330,6 @@ static int reset_flow_steering(void)
> run_command(
> "ethtool -n %s | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N %s delete >&2",
> ifname, ifname);
> - return 0;
> }
>
> static const char *tcp_data_split_str(int val)
> @@ -354,6 +391,11 @@ static int configure_rss(void)
> return run_command("ethtool -X %s equal %d >&2", ifname, start_queue);
> }
>
> +static void reset_rss(void)
> +{
> + run_command("ethtool -X %s default >&2", ifname, start_queue);
> +}
> +
> static int configure_channels(unsigned int rx, unsigned int tx)
> {
> struct ethtool_channels_get_req *gchan;
> @@ -479,6 +521,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
>
> *ys = ynl_sock_create(&ynl_netdev_family, &yerr);
> if (!*ys) {
> + netdev_queue_id_free(queues);
Funny how you spotted this.. Ownership of these is complicated with ynl :-(
Powered by blists - more mailing lists