[{"id":3685456,"web_url":"http://patchwork.ozlabs.org/comment/3685456/","msgid":"","list_archive_url":null,"date":"2026-05-03T20:56:19","subject":"Re: [PATCH] tools: gpio: fix buffer overflow and add bounds check","submitter":{"id":93193,"url":"http://patchwork.ozlabs.org/api/people/93193/","name":"Maxwell Doose","email":"m32285159@gmail.com"},"content":"On Sun May 3, 2026 at 2:00 PM CDT, Zhang Xiaolei wrote:\n> Replace strcpy() with strncpy() to avoid potential buffer overflow\n> in req.consumer. Also add validation for num_lines to prevent\n> out-of-bounds access to req.offsets.\n>\n> Fix incorrect ioctl name in error message.\n>\n> Signed-off-by: Zhang Xiaolei \n> ---\n> tools/gpio/gpio-utils.c | 14 ++++++++++----\n> 1 file changed, 10 insertions(+), 4 deletions(-)\n>\n\nThis looks like a patch that could be split, since you're adding\nNULL pointer checking, a potential overflow fix, and a style fix all\ninto one. I like the idea, but we need to split functional changes.\n\n>\n> diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c\n> index 4096bcd511d1..1afd9dff2bed 100644\n> --- a/tools/gpio/gpio-utils.c\n> +++ b/tools/gpio/gpio-utils.c\n> @@ -65,11 +65,15 @@ int gpiotools_request_line(const char *device_name, unsigned int *lines,\n> \tint i;\n> \tint ret;\n> \n> +\tif (!device_name || !lines || !config || !consumer ||\n> +\t num_lines == 0 || num_lines > GPIO_V2_LINES_MAX)\n> +\t\treturn -EINVAL;\n> +\n>\n\nFirst off, this can be split into a different patch. Secondly, I feel as\nif we should make two if statements for this, one for the NULL and 0\nchecking and the other for checking num_lines. And returning -EINVAL\nwill be ambiguous for the caller, so maybe change that.\n\n>\n> \tret = asprintf(&chrdev_name, \"/dev/%s\", device_name);\n> \tif (ret < 0)\n> \t\treturn -ENOMEM;\n> \n> -\tfd = open(chrdev_name, 0);\n> +\tfd = open(chrdev_name, O_RDONLY);\n>\n\nAnother thing that can be split.\n\n>\n> \tif (fd == -1) {\n> \t\tret = -errno;\n> \t\tfprintf(stderr, \"Failed to open %s, %s\\n\",\n> @@ -78,27 +82,29 @@ int gpiotools_request_line(const char *device_name, unsigned int *lines,\n> \t}\n> \n> \tmemset(&req, 0, sizeof(req));\n> +\n>\n\nThis seems like a stray change.\n\n>\n> \tfor (i = 0; i < num_lines; i++)\n> \t\treq.offsets[i] = lines[i];\n> \n> \treq.config = *config;\n> -\tstrcpy(req.consumer, consumer);\n> +\tstrncpy(req.consumer, consumer, sizeof(req.consumer) - 1);\n> +\treq.consumer[sizeof(req.consumer) - 1] = '\\0';\n>\n\nWe already invented a solution for this in the form of strscpy(), so\nplease change this to use that instead, something like:\n\n\tstrscpy(req.consumer, consumer, sizeof(req.consumer));\n\n>\n> \treq.num_lines = num_lines;\n> \n> \tret = ioctl(fd, GPIO_V2_GET_LINE_IOCTL, &req);\n> \tif (ret == -1) {\n> \t\tret = -errno;\n> \t\tfprintf(stderr, \"Failed to issue %s (%d), %s\\n\",\n> -\t\t\t\"GPIO_GET_LINE_IOCTL\", ret, strerror(errno));\n> +\t\t\t\"GPIO_V2_GET_LINE_IOCTL\", ret, strerror(errno));\n>\n\nAnother thing that could be split.\n\n>\n> \t}\n> \n> \tif (close(fd) == -1)\n> \t\tperror(\"Failed to close GPIO character device file\");\n> +\n>\n\nMight be another stray change.\n\n>\n> exit_free_name:\n> \tfree(chrdev_name);\n> \treturn ret < 0 ? ret : req.fd;\n> }\n> -\n>\n\nMaybe another stray change?\n\n>\n> /**\n> * gpiotools_set_values() - Set the value of gpio(s)\n> * @fd:\t\t\tThe fd returned by\n\nI like the idea, but we should be splitting some of these changes to\nfollow the atomic commits idea of the kernel.\n\nbest regards,\nmaxwell","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-gpio@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20251104 header.b=RCYHiwkg;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-gpio+bounces-36003-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=\"RCYHiwkg\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=209.85.210.46","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=gmail.com"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g7xs80bMXz1yJV\n\tfor ; Mon, 04 May 2026 06:56:27 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 9A9B230078D5\n\tfor ; Sun, 3 May 2026 20:56:24 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 3624F39022A;\n\tSun, 3 May 2026 20:56:23 +0000 (UTC)","from mail-ot1-f46.google.com (mail-ot1-f46.google.com\n [209.85.210.46])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id A6A243559F5\n\tfor ; Sun, 3 May 2026 20:56:21 +0000 (UTC)","by mail-ot1-f46.google.com with SMTP id\n 46e09a7af769-7dbcb467f2bso3281797a34.3\n for ;\n Sun, 03 May 2026 13:56:21 -0700 (PDT)","from localhost (23-88-128-2.fttp.usinternet.com. [23.88.128.2])\n by smtp.gmail.com with ESMTPSA id\n 006d021491bc7-69689440f5asm5146886eaf.1.2026.05.03.13.56.19\n (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n Sun, 03 May 2026 13:56:20 -0700 (PDT)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777841783; cv=none;\n b=sARJSQhvEK5DRxdxl/DeCH4LgC3J+jUfxeRxcZs9jtSuczmWLkeOripoKat7QY803wuEjquU3vdMJL/DmDTdU8buBqPUiQz0cNeYzmki1BzMB4Jxfqm8VoGbrbpxc/w6wcl25vNiZ86/nsWLmQ3RnWd+Lpu878IveLdmQJtF6j4=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777841783; c=relaxed/simple;\n\tbh=X882GCO+Dvlf2ebX1aZh7TkhwiTLTaW9iuH+RMqeYYU=;\n\th=Mime-Version:Content-Type:Date:Message-Id:Subject:From:To:Cc:\n\t References:In-Reply-To;\n b=SYe4F2t8CY6Xm8PrLsdiQnpOOI2E2Z8QXc9l8AkbAwLCpkSiox848q0QKAXQz75SOS+oeY9XddGL0HZo3cvVkB00H+gz4L0OVT2i5s4r3Wzm8Q8Waxsmq9/IpHHOG+qr66UeRSPEo51gr+nEj/RF2+fRHEz0UWN00HUrEuVX/aQ=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com;\n spf=pass smtp.mailfrom=gmail.com;\n dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=RCYHiwkg; arc=none smtp.client-ip=209.85.210.46","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20251104; t=1777841780; x=1778446580;\n darn=vger.kernel.org;\n h=in-reply-to:references:cc:to:from:subject:message-id:date\n :content-transfer-encoding:mime-version:from:to:cc:subject:date\n :message-id:reply-to;\n bh=s4qRCFUgyMlFXryUo+UrBwsmSJQu18VkeGRzbCwuX5w=;\n b=RCYHiwkgXaOzIgEKpsfEnRCJKhkVgv7iZuOIrOQEClbxmxCnZ76zMLMc5B8thQaVAG\n CAYsSLiUsnCUY9R1m0GdLWYRUMCsTOcPww+ju930pjpDtG3ZDKhpETdsm9JJnnqxLWvA\n dAVIWMqFJOb4aGV79pIL+6pkqKncNVmH9wwCBdyQve65dcGeKF3pqiY64f2ObeaWBkne\n yySlcLDT5wZZOn1zS2Uwuury7533exEsZXvoMac82T1Al/WUTmnv3SXgubN9bFD20CGo\n 2LLAHZjYDZyKDqcQEa32Au3KgHt8Dmc+31zRkRR/XC3e/r7/fKbi89mrTDDQCQIeLJN8\n X2NA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1777841780; x=1778446580;\n h=in-reply-to:references:cc:to:from:subject:message-id:date\n :content-transfer-encoding:mime-version:x-gm-gg:x-gm-message-state\n :from:to:cc:subject:date:message-id:reply-to;\n bh=s4qRCFUgyMlFXryUo+UrBwsmSJQu18VkeGRzbCwuX5w=;\n b=FYoYhOvv9ytnuJc5jffObjFx8J1Ez8J5i0pauV4vsQUoamoVRvtip50GbKEU8MN6Zq\n aGOJlF0ZAlmBq0o1HF22fVuzgUV8BBzvi7qW3nmAdhdy/1FwlGfUsw7W7MPOmVavVNpE\n 8wV6TW3BOVQckLradWc6e46bX5V53u1HT9dY2sKSVK4zgg7yBFAPkeJO8TjEAGhnOCqx\n Gwp1nhbJ0H4avLrf4ZAn6uFWAhNwXBpTV3IwlwGGkom4SftFb+37dujgeOqjDIbxY+qo\n Hycyp1urGBTCfXHi1cJuv0YZJAIcLwO8u/SFAt6P+NTF+aDTGtinZm40Ho1x9eE+v5Fe\n nHcA==","X-Forwarded-Encrypted":"i=1;\n AFNElJ8kKB0nIo/NgGuoLWVwlYpci3f/vtQbks0p3vvpx+5bg/bz15j+RytDpA/iGUFjZDeXsNWO/WoPBBLI@vger.kernel.org","X-Gm-Message-State":"AOJu0YwgcIzPJAtXvxdsugeyblLXoOscB6B7bT13E4rT56qmAKTPMpK6\n\tes2OSRdsK2JvN1PUzhGREzYRDMwE2fSXKu3wCjr4bFvCsRUX+731CrJ/","X-Gm-Gg":"AeBDietYLecmOLr3+dsa1zGtjlAfnuKBmjC8hseVzj0p9M6UoY/HpBSgUanH/YY+h+W\n\t8kdGCWn2h7m/9t2t/iugaG7znYsMsW6x7juM+4HEFXBhUAmLLxbDntgn8GLd5Go/oaLvvsb7ahc\n\tc026AlWm378b6bRWLhIt98LMmfTLYdyxDTd4bTgYHlWDGahkIjPMQ0OwpnBai7eWlqvma4mij4e\n\tEm23pCusYfUAcMx5fjK6ljCo7NQysqeYJr8+53mAU3M+dZdcCSXw4vYul9DPPKqhhJlg2tP+8/R\n\tiTnz24/x3hRir8marCMx2vnVzB2L1G0JuX/8ESs7OMtr22w3XiQaBOS9XUnrZuOt7M9fKn36Lty\n\tH0o26cjuqmH8QmODGQ0qJTKYtC29yNOhxgMADfTMxPLzgCnf94prT5CSngq8EdTA16Zc+I0N6rz\n\tz8/qeaT2VU5ofd8Rjcpm+Ed2ZDxqi4pvuciQNsBQnD/7s7Q7FYqV/HfuYEaFJZBzhk0ks4rnw=","X-Received":"by 2002:a05:6820:1888:b0:68e:3f3b:8690 with SMTP id\n 006d021491bc7-696979feb08mr3582426eaf.21.1777841780608;\n Sun, 03 May 2026 13:56:20 -0700 (PDT)","Precedence":"bulk","X-Mailing-List":"linux-gpio@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","Mime-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=UTF-8","Date":"Sun, 03 May 2026 15:56:19 -0500","Message-Id":"","Subject":"Re: [PATCH] tools: gpio: fix buffer overflow and add bounds check","From":"\"Maxwell Doose\" ","To":"\"Zhang Xiaolei\" , ","Cc":", ,\n ","X-Mailer":"aerc 0.21.0-0-g5549850facc2","References":"<20260503190016.13439-1-zxl434815272@gmail.com>","In-Reply-To":"<20260503190016.13439-1-zxl434815272@gmail.com>"}},{"id":3685501,"web_url":"http://patchwork.ozlabs.org/comment/3685501/","msgid":"<81958e85-da7b-4b4d-927b-f7515a2b02a3@gmail.com>","list_archive_url":null,"date":"2026-05-04T05:46:02","subject":"Re: [PATCH] tools: gpio: fix buffer overflow and add bounds check","submitter":{"id":93314,"url":"http://patchwork.ozlabs.org/api/people/93314/","name":"Zhang Xiaolei","email":"zxl434815272@gmail.com"},"content":"Hi Maxwell,\n\n > This looks like a patch that could be split, since you're adding\n > NULL pointer checking, a potential overflow fix, and a style fix all\n > into one. I like the idea, but we need to split functional changes.\n\nThanks for the review. I agree that these changes should be split into\nsmaller atomic patches. I will prepare a v2 series.\n\n > First off, this can be split into a different patch. Secondly, I feel as\n > if we should make two if statements for this, one for the NULL and 0\n > checking and the other for checking num_lines. And returning -EINVAL\n > will be ambiguous for the caller, so maybe change that.\n\nGot it. I will split the argument validation and the num_lines bounds\ncheck, and reconsider the return values.\n\n > Another thing that can be split.\n\nI will drop the O_RDONLY change from v2 unless it is useful as a separate\ncleanup.\n\n > This seems like a stray change.\n\nThanks, I will remove unrelated whitespace changes.\n\n > We already invented a solution for this in the form of strscpy(), so\n > please change this to use that instead\n\nAgreed. I will use strscpy() in v2.\n\n > Another thing that could be split.\n\nI will split the ioctl name fix into a separate patch.\n\nThanks,\nZhang Xiaolei\n\nOn 5/4/26 04:56, Maxwell Doose wrote:\n> On Sun May 3, 2026 at 2:00 PM CDT, Zhang Xiaolei wrote:\n>> Replace strcpy() with strncpy() to avoid potential buffer overflow\n>> in req.consumer. Also add validation for num_lines to prevent\n>> out-of-bounds access to req.offsets.\n>>\n>> Fix incorrect ioctl name in error message.\n>>\n>> Signed-off-by: Zhang Xiaolei \n>> ---\n>> tools/gpio/gpio-utils.c | 14 ++++++++++----\n>> 1 file changed, 10 insertions(+), 4 deletions(-)\n>>\n> This looks like a patch that could be split, since you're adding\n> NULL pointer checking, a potential overflow fix, and a style fix all\n> into one. I like the idea, but we need to split functional changes.\n>\n>> diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c\n>> index 4096bcd511d1..1afd9dff2bed 100644\n>> --- a/tools/gpio/gpio-utils.c\n>> +++ b/tools/gpio/gpio-utils.c\n>> @@ -65,11 +65,15 @@ int gpiotools_request_line(const char *device_name, unsigned int *lines,\n>> \tint i;\n>> \tint ret;\n>> \n>> +\tif (!device_name || !lines || !config || !consumer ||\n>> +\t num_lines == 0 || num_lines > GPIO_V2_LINES_MAX)\n>> +\t\treturn -EINVAL;\n>> +\n>>\n> First off, this can be split into a different patch. Secondly, I feel as\n> if we should make two if statements for this, one for the NULL and 0\n> checking and the other for checking num_lines. And returning -EINVAL\n> will be ambiguous for the caller, so maybe change that.\n>\n>> \tret = asprintf(&chrdev_name, \"/dev/%s\", device_name);\n>> \tif (ret < 0)\n>> \t\treturn -ENOMEM;\n>> \n>> -\tfd = open(chrdev_name, 0);\n>> +\tfd = open(chrdev_name, O_RDONLY);\n>>\n> Another thing that can be split.\n>\n>> \tif (fd == -1) {\n>> \t\tret = -errno;\n>> \t\tfprintf(stderr, \"Failed to open %s, %s\\n\",\n>> @@ -78,27 +82,29 @@ int gpiotools_request_line(const char *device_name, unsigned int *lines,\n>> \t}\n>> \n>> \tmemset(&req, 0, sizeof(req));\n>> +\n>>\n> This seems like a stray change.\n>\n>> \tfor (i = 0; i < num_lines; i++)\n>> \t\treq.offsets[i] = lines[i];\n>> \n>> \treq.config = *config;\n>> -\tstrcpy(req.consumer, consumer);\n>> +\tstrncpy(req.consumer, consumer, sizeof(req.consumer) - 1);\n>> +\treq.consumer[sizeof(req.consumer) - 1] = '\\0';\n>>\n> We already invented a solution for this in the form of strscpy(), so\n> please change this to use that instead, something like:\n>\n> \tstrscpy(req.consumer, consumer, sizeof(req.consumer));\n>\n>> \treq.num_lines = num_lines;\n>> \n>> \tret = ioctl(fd, GPIO_V2_GET_LINE_IOCTL, &req);\n>> \tif (ret == -1) {\n>> \t\tret = -errno;\n>> \t\tfprintf(stderr, \"Failed to issue %s (%d), %s\\n\",\n>> -\t\t\t\"GPIO_GET_LINE_IOCTL\", ret, strerror(errno));\n>> +\t\t\t\"GPIO_V2_GET_LINE_IOCTL\", ret, strerror(errno));\n>>\n> Another thing that could be split.\n>\n>> \t}\n>> \n>> \tif (close(fd) == -1)\n>> \t\tperror(\"Failed to close GPIO character device file\");\n>> +\n>>\n> Might be another stray change.\n>\n>> exit_free_name:\n>> \tfree(chrdev_name);\n>> \treturn ret < 0 ? ret : req.fd;\n>> }\n>> -\n>>\n> Maybe another stray change?\n>\n>> /**\n>> * gpiotools_set_values() - Set the value of gpio(s)\n>> * @fd:\t\t\tThe fd returned by\n> I like the idea, but we should be splitting some of these changes to\n> follow the atomic commits idea of the kernel.\n>\n> best regards,\n> maxwell","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-gpio@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20251104 header.b=jA0YhjvF;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-gpio+bounces-36004-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=\"jA0YhjvF\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=209.85.215.174","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=gmail.com"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g89cN6KCkz1yJ9\n\tfor ; Mon, 04 May 2026 15:46:12 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 5DE713007C87\n\tfor ; Mon, 4 May 2026 05:46:09 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 8348523E334;\n\tMon, 4 May 2026 05:46:08 +0000 (UTC)","from mail-pg1-f174.google.com (mail-pg1-f174.google.com\n [209.85.215.174])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 117954086A\n\tfor ; Mon, 4 May 2026 05:46:06 +0000 (UTC)","by mail-pg1-f174.google.com with SMTP id\n 41be03b00d2f7-c70c112cb61so2102409a12.0\n for ;\n Sun, 03 May 2026 22:46:06 -0700 (PDT)","from ?IPV6:2402:f000:2:4001:427f:2eb3:870d:a19d?\n ([2402:f000:2:4001:427f:2eb3:870d:a19d])\n by smtp.gmail.com with ESMTPSA id\n 41be03b00d2f7-c7ffbc6f063sm8457392a12.20.2026.05.03.22.46.04\n (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n Sun, 03 May 2026 22:46:06 -0700 (PDT)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777873568; cv=none;\n b=Ofp8SES8x9b6HPrmrMhWw3fS0CAIlXkfh1nkGN5TTdLLLEDVcafbOepU8ILNV+UUa6j4cLMjOV+hRZy13SsaLTDYIraX3ShCqiiu28bNbdQYXPHFWFYyTq0THHoTIgJOuwVhw/BNq28iRhtE/auQljzDsPczoDVFEEsch/UehvE=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777873568; c=relaxed/simple;\n\tbh=9JqMPn0RXpz+fcD36D3dFToJCRezEpPBGiispDmrjPk=;\n\th=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:\n\t In-Reply-To:Content-Type;\n b=axyKKRZ8vJiB9Q0lDK9QbNh5gEEb0XCA4aOWLMluDI7Fviy4HlQ1ZFBULpktaXANM8K+w3JzWldknXzjni1rItEKf+4ZhAXXMyhJnU9yFX83GfipMLpmzrOy7pxQeulR0O2DQ9IxdsJ5UuHOrNyy8OTsdnwnl3xVap7t8oGTvps=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com;\n spf=pass smtp.mailfrom=gmail.com;\n dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=jA0YhjvF; arc=none smtp.client-ip=209.85.215.174","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20251104; t=1777873566; x=1778478366;\n darn=vger.kernel.org;\n h=content-transfer-encoding:in-reply-to:from:content-language\n :references:cc:to:subject:user-agent:mime-version:date:message-id\n :from:to:cc:subject:date:message-id:reply-to;\n bh=JMdrqXAeZlHZMAjNWM5ngvOGBdh+mR1QfsCscY7ufQo=;\n b=jA0YhjvF1Ld81pQIpOJeHUPNpOwHVIeJ1r1/t3HX1NJ8UNJQZMVAmp3fFCQPCsEf7b\n NfqDGuay8vtp8EwO/NYpazjV3Gs7guJihDKai2Ah5K1/D50iujD+7xZgqUWx4N/H1qQM\n bKU+sf050EcJ5Zu/izSzmmr2eTlEHudcolQmLD3MoTiGRuK5VOfqj4A8zaAg1OGFnUIF\n aL7DTGLLP7/6VenhDulD3AG/sPB2T/TEPDqJxP4DSm6QJ20HHvXHg93H6IvehWvUop4T\n fnJuZbdmtjHAoi1WU3GwDGR4obZI2V2H72v5R05/sgul3He+iu/7WVEoIHoDxgn71e7j\n WhaQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1777873566; x=1778478366;\n h=content-transfer-encoding:in-reply-to:from:content-language\n :references:cc:to:subject:user-agent:mime-version:date:message-id\n :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id\n :reply-to;\n bh=JMdrqXAeZlHZMAjNWM5ngvOGBdh+mR1QfsCscY7ufQo=;\n b=Ii7iHGseaURK2ojfUefJ4OSBONYPeEv3pSQT6MfSu6dlerHFvn94y50qdDZZsebdlL\n 8yO5RasiWbtJLEyshWhX96cgM6Kucsoal0MhNPHyL0o/7wUYs+H7zUQOF4sVQv2ZKRjL\n ZTseYP/srv/ThaLT5UcVJ8Nzg/fTLVTv6jWcfMbBqKBksRkOJCPH62u60H4cqv3p00ol\n ihsnL2N8b/AQqs0u1ybr0jjjOph04Bv+pQDSg6S6EGmuJaoqQhC1LUMsAVfpzuxcQPs9\n TbS9WPg+R8xBhBnndvIwnQlaYEjAcf63P+yhfeBKxnkhr/e7DrZs3JTE/XmJjOOeTQxo\n 3NJw==","X-Forwarded-Encrypted":"i=1;\n AFNElJ+hmUh1KzDEvZdsDF8+dI+8Z3H/aTrvWtWbDF3ijAC7JpT+r1dH0WHa8XdlgXjq7xdHZds4YU6xsWn8@vger.kernel.org","X-Gm-Message-State":"AOJu0YzXxBFteGmiGadwxY8MhhSMljShTM//5+zm2rHMUunyz5edkiUi\n\ttVnrqVLAeqL3k32UfqtPyky9f6oCVjqxidJfdFWxv8RA/MmyQGZjivH1","X-Gm-Gg":"AeBDiesm8alUAmsdGxI6Zn0u0d7jxorUg+Os1yGlYgYopD4+gAl6E7Cwkkvy2zAS5cd\n\tZL+KJACs+yrF097iLRBS9l6p18nwAUP8D/23Jk62js9m3VqEO9+csL27Xghe4fjjbZAwq9hlUpL\n\tQtSksMYIq3iIK+vK4Prfc5uqa/ukqlK21OV5GMxPxVgSjCI6mbfqD4MW58KfPm3VEo/kd2xSrK5\n\tVik/GveIrMqu1DiXP8glhCF2UKjRf0mNZRw1nvJ0tfMgGqYFIpf7b6qjwh2X0MkQFADHbXMCy0m\n\twzl/QdSzaFkdzhsyGEjY1azynrukbuvUS0IKpX5A/BSYSRKKSsBRtN8KzppAq3f0uD8ktVLOpYP\n\tA2LfVUl/wUS31inGP1AT9GXScI9IrM5gR8CNt5uSL7fsCvZpXeXTmBddzolg4kC2CxZaJDRiV4f\n\tonjBgzMIeyziJN5IaPOgfi7zqHrUNxevazzkpdl3NVFCFfBRnMYLUSuvipW6/dj2HHrGABFjE86\n\tsEDO2A=","X-Received":"by 2002:a05:6a20:3d84:b0:398:8002:8033 with SMTP id\n adf61e73a8af0-3a7f1fce0a8mr9056079637.49.1777873566326;\n Sun, 03 May 2026 22:46:06 -0700 (PDT)","Message-ID":"<81958e85-da7b-4b4d-927b-f7515a2b02a3@gmail.com>","Date":"Mon, 4 May 2026 13:46:02 +0800","Precedence":"bulk","X-Mailing-List":"linux-gpio@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] tools: gpio: fix buffer overflow and add bounds check","To":"Maxwell Doose , linux-gpio@vger.kernel.org","Cc":"brgl@kernel.org, warthog618@gmail.com, linux-kernel@vger.kernel.org","References":"<20260503190016.13439-1-zxl434815272@gmail.com>\n ","Content-Language":"en-US","From":"007 ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit"}},{"id":3685504,"web_url":"http://patchwork.ozlabs.org/comment/3685504/","msgid":"<75dda615-d4c3-4e14-8c5b-736a42fe4442@gmail.com>","list_archive_url":null,"date":"2026-05-04T05:55:52","subject":"Re: [PATCH] tools: gpio: fix buffer overflow and add bounds check","submitter":{"id":93314,"url":"http://patchwork.ozlabs.org/api/people/93314/","name":"Zhang Xiaolei","email":"zxl434815272@gmail.com"},"content":"Hi Maxwell,\n\n> This looks like a patch that could be split, since you're adding\n> NULL pointer checking, a potential overflow fix, and a style fix all\n> into one. I like the idea, but we need to split functional changes.\n\nThanks for the review. I agree that these changes should be split into\nsmaller atomic patches. I will prepare a v2 series.\n\n> First off, this can be split into a different patch. Secondly, I feel as\n> if we should make two if statements for this, one for the NULL and 0\n> checking and the other for checking num_lines. And returning -EINVAL\n> will be ambiguous for the caller, so maybe change that.\n\nGot it. I will split the argument validation and the num_lines bounds\ncheck, and reconsider the return values.\n\n> Another thing that can be split.\n\nI will drop the O_RDONLY change from v2 unless it is useful as a separate\ncleanup.\n\n> This seems like a stray change.\n\nThanks, I will remove unrelated whitespace changes.\n\n> We already invented a solution for this in the form of strscpy(), so\n> please change this to use that instead\n\nAgreed. I will use strscpy() in v2.\n\n> Another thing that could be split.\n\nI will split the ioctl name fix into a separate patch.\n\nThanks,\nZhang Xiaolei\n\nOn 5/4/26 04:56, Maxwell Doose wrote:\n> On Sun May 3, 2026 at 2:00 PM CDT, Zhang Xiaolei wrote:\n>> Replace strcpy() with strncpy() to avoid potential buffer overflow\n>> in req.consumer. Also add validation for num_lines to prevent\n>> out-of-bounds access to req.offsets.\n>>\n>> Fix incorrect ioctl name in error message.\n>>\n>> Signed-off-by: Zhang Xiaolei \n>> ---\n>> tools/gpio/gpio-utils.c | 14 ++++++++++----\n>> 1 file changed, 10 insertions(+), 4 deletions(-)\n>>\n> This looks like a patch that could be split, since you're adding\n> NULL pointer checking, a potential overflow fix, and a style fix all\n> into one. I like the idea, but we need to split functional changes.\n>\n>> diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c\n>> index 4096bcd511d1..1afd9dff2bed 100644\n>> --- a/tools/gpio/gpio-utils.c\n>> +++ b/tools/gpio/gpio-utils.c\n>> @@ -65,11 +65,15 @@ int gpiotools_request_line(const char *device_name, unsigned int *lines,\n>> \tint i;\n>> \tint ret;\n>> \n>> +\tif (!device_name || !lines || !config || !consumer ||\n>> +\t num_lines == 0 || num_lines > GPIO_V2_LINES_MAX)\n>> +\t\treturn -EINVAL;\n>> +\n>>\n> First off, this can be split into a different patch. Secondly, I feel as\n> if we should make two if statements for this, one for the NULL and 0\n> checking and the other for checking num_lines. And returning -EINVAL\n> will be ambiguous for the caller, so maybe change that.\n>\n>> \tret = asprintf(&chrdev_name, \"/dev/%s\", device_name);\n>> \tif (ret < 0)\n>> \t\treturn -ENOMEM;\n>> \n>> -\tfd = open(chrdev_name, 0);\n>> +\tfd = open(chrdev_name, O_RDONLY);\n>>\n> Another thing that can be split.\n>\n>> \tif (fd == -1) {\n>> \t\tret = -errno;\n>> \t\tfprintf(stderr, \"Failed to open %s, %s\\n\",\n>> @@ -78,27 +82,29 @@ int gpiotools_request_line(const char *device_name, unsigned int *lines,\n>> \t}\n>> \n>> \tmemset(&req, 0, sizeof(req));\n>> +\n>>\n> This seems like a stray change.\n>\n>> \tfor (i = 0; i < num_lines; i++)\n>> \t\treq.offsets[i] = lines[i];\n>> \n>> \treq.config = *config;\n>> -\tstrcpy(req.consumer, consumer);\n>> +\tstrncpy(req.consumer, consumer, sizeof(req.consumer) - 1);\n>> +\treq.consumer[sizeof(req.consumer) - 1] = '\\0';\n>>\n> We already invented a solution for this in the form of strscpy(), so\n> please change this to use that instead, something like:\n>\n> \tstrscpy(req.consumer, consumer, sizeof(req.consumer));\n>\n>> \treq.num_lines = num_lines;\n>> \n>> \tret = ioctl(fd, GPIO_V2_GET_LINE_IOCTL, &req);\n>> \tif (ret == -1) {\n>> \t\tret = -errno;\n>> \t\tfprintf(stderr, \"Failed to issue %s (%d), %s\\n\",\n>> -\t\t\t\"GPIO_GET_LINE_IOCTL\", ret, strerror(errno));\n>> +\t\t\t\"GPIO_V2_GET_LINE_IOCTL\", ret, strerror(errno));\n>>\n> Another thing that could be split.\n>\n>> \t}\n>> \n>> \tif (close(fd) == -1)\n>> \t\tperror(\"Failed to close GPIO character device file\");\n>> +\n>>\n> Might be another stray change.\n>\n>> exit_free_name:\n>> \tfree(chrdev_name);\n>> \treturn ret < 0 ? ret : req.fd;\n>> }\n>> -\n>>\n> Maybe another stray change?\n>\n>> /**\n>> * gpiotools_set_values() - Set the value of gpio(s)\n>> * @fd:\t\t\tThe fd returned by\n> I like the idea, but we should be splitting some of these changes to\n> follow the atomic commits idea of the kernel.\n>\n> best regards,\n> maxwell","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-gpio@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20251104 header.b=cpeGvX2V;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.234.253.10; helo=sea.lore.kernel.org;\n envelope-from=linux-gpio+bounces-36005-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=\"cpeGvX2V\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=209.85.216.41","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=gmail.com"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org [172.234.253.10])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g89qt36ZGz1yJV\n\tfor ; Mon, 04 May 2026 15:56:10 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 51D96300EFB0\n\tfor ; Mon, 4 May 2026 05:56:00 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 3A0202853FD;\n\tMon, 4 May 2026 05:55:59 +0000 (UTC)","from mail-pj1-f41.google.com (mail-pj1-f41.google.com\n [209.85.216.41])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 8388A2797AC\n\tfor ; Mon, 4 May 2026 05:55:56 +0000 (UTC)","by mail-pj1-f41.google.com with SMTP id\n 98e67ed59e1d1-358ed696623so1615806a91.0\n for ;\n Sun, 03 May 2026 22:55:56 -0700 (PDT)","from ?IPV6:2402:f000:2:4001:944:3690:ccf6:2932?\n ([2402:f000:2:4001:944:3690:ccf6:2932])\n by smtp.gmail.com with ESMTPSA id\n d2e1a72fcca58-835158bd62fsm9894938b3a.24.2026.05.03.22.55.54\n (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n Sun, 03 May 2026 22:55:55 -0700 (PDT)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777874159; cv=none;\n b=aUZZqsDa4407IXTJljfYnQmZwurGeSyb9GSjzjoyMki1d1AEHIqHgBahKGIHo9K5QPEJTLfBi76nE0E2CB2XJ3KShPJ6o58MmcKkjytFvCxOXNAegnZk2uoHQdzSRQ8FuarGdzXbAfqiz6UID67ts+gGPU9zwoNXKOSakwGfalI=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777874159; c=relaxed/simple;\n\tbh=0JYb+47SuTUvKMIjNtlWIKxuIpEYewG2doFKKx6vhiE=;\n\th=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References:\n\t In-Reply-To:Content-Type;\n b=T22gFzSo0HioFUQcUvk4ytJf4tOkQNdmgtFb2isdTdtHTloHtR6P/TeRKiEBGJasc7tSbp1wXFkxR9Bf6+RSLVOiQ1baYvHtEwMVp6ox7LJiyEASTQSQm7QgsKUjpDY/K/LCJeQCQmooi2gjyxeK35Kg/U4M98g5FZwfi4ZWHOc=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com;\n spf=pass smtp.mailfrom=gmail.com;\n dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=cpeGvX2V; arc=none smtp.client-ip=209.85.216.41","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=gmail.com; s=20251104; t=1777874156; x=1778478956;\n darn=vger.kernel.org;\n h=content-transfer-encoding:in-reply-to:content-language:references\n :cc:to:subject:from:user-agent:mime-version:date:message-id:from:to\n :cc:subject:date:message-id:reply-to;\n bh=qSbGg0k0BvR8bHpFwu2SlmO/vZ7U24qUHDBvD74bJMk=;\n b=cpeGvX2VD+eKwqRK/mTAQSu7JYqkRVOgDCHPK7JH/eHxK6YfwTqSONZhoGh8J6G2sL\n d1pJ7HtPRF2NpKOvB6dM5bvdyhPOT6FeXCnfD0ZhfZTRJJKcSxMxRuJyhg+UqOT6a7SE\n 5PM9aIUYDLmrPnJlvrD9veY2YdPEsR2ohRPSIygOVqzbwwEVF4KzaiCa8Z6/Bdt1IPJb\n WZ1SGmedNImDOdcV6ZtMSiNYv4E20zc8getbujpVSNyZuUcg4D/EeVwvvfTnOVxHB8vU\n R/652r/kbzLE4hpkAWjTLK9aRxwKOzmBC97xAkKmo7d60tzzd0fDeWny47Tpm7Yv4WM2\n 9mDg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20251104; t=1777874156; x=1778478956;\n h=content-transfer-encoding:in-reply-to:content-language:references\n :cc:to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg\n :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n bh=qSbGg0k0BvR8bHpFwu2SlmO/vZ7U24qUHDBvD74bJMk=;\n b=dE1l71IKCSOwhHLMtVwXeJkU2SVkKhu0TsN5gIR0UT2/noP+h9C/oUFQHQ05XzWnUP\n qMjHzdKDi+h0BJQJ70M4VQujLjJVPdQYC/bxKf1wxfYNxZVIqtvZWPPIPbLnPcmmw3Wi\n an9mfvtOtJNjz3SVVx+W/2idMsk1gKQ6fGDoYhJphMlPsRYccxcD5abq7CJMNybgVLhs\n +OHgwCU+1c5BeuQaRbJJkcrnGEcwb1hHBxeoaHSlIFXnstt+fjRYgHvY4x/5ilh3xEJ/\n 3LRqmZ/rOrKrwapkDzBeUormEFqrNKptB0n2BhFeq7wy55JL7VbcoHP8NEXoNwz83tHJ\n 9q/Q==","X-Forwarded-Encrypted":"i=1;\n AFNElJ/RQ3KUhWYXZUIZhtW6xFmexeCBMPYkSLtkTcLMO13B5vYxUqe5lvi0pHYYZ5X3FW210K3TNDHPcTP6@vger.kernel.org","X-Gm-Message-State":"AOJu0YxDI7Y20+MEiiKd0YhoVjQBeL0T5Nmxs27TnpDQscJ8eG8qEWfB\n\tDTAF3lHkQIvL3hc+JICdgIRVoFQIxdvLf7ziAhwoIOzJezsCI++WUjK/","X-Gm-Gg":"AeBDievdOlAcTjGelEpME7vF+OOS3hShTYRYPXekCSGVieY0BP04A7IBLBX/Z7xkeLG\n\t///Jx6tg1rXxZem2oRuyWbzxCM24DqTCsbI5W67deeQs/hEyaXu0AkqYSyu6b+sP+5jh79oYN4G\n\t1QNonc/xfRYcOnbpy4lif/Dqu43/otNXSR4dFlIIQorX2rJNXqrRe9t0WnRSXbP+XlDZ/hY77Lq\n\t59hQD00nR0wxbb9R15j1eOVAvXO8fosIvurYrb2rTFi+1g5/bCnfr9uhNTuNVVw60Z1xc9jWKlM\n\tZ8thVzATLM8epIFHejEotDa0L/d5+XgDqfAnK47/TZArcbI0tu3nvhu7rVj3B8zbxBLx3LEH/4e\n\t6Ao8SGL7X3ppjtpXKyXDxf5ArSep3Q1LKfsvG1k49mAix8i4T+1tufPYI3IKWzvZf+K7YBY751g\n\td1mgaYYFlD0EzxghbppXNlDMfaSFL8T0orBetalgrHvIWObEKU4uNheo88X9f+baQpu1Fpb/YOO\n\tm6p","X-Received":"by 2002:a05:6a20:9392:b0:3a3:a5cd:560d with SMTP id\n adf61e73a8af0-3a7f1a1df35mr8814589637.9.1777874155812;\n Sun, 03 May 2026 22:55:55 -0700 (PDT)","Message-ID":"<75dda615-d4c3-4e14-8c5b-736a42fe4442@gmail.com>","Date":"Mon, 4 May 2026 13:55:52 +0800","Precedence":"bulk","X-Mailing-List":"linux-gpio@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","From":"007 ","Subject":"Re: [PATCH] tools: gpio: fix buffer overflow and add bounds check","To":"Maxwell Doose , linux-gpio@vger.kernel.org","Cc":"brgl@kernel.org, warthog618@gmail.com, linux-kernel@vger.kernel.org","References":"<20260503190016.13439-1-zxl434815272@gmail.com>\n ","Content-Language":"en-US","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit"}}]