From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C376142A59; Thu, 4 May 2023 09:37:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 574D74021F; Thu, 4 May 2023 09:37:02 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 7369C40141 for ; Thu, 4 May 2023 09:37:00 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 223246404 for ; Thu, 4 May 2023 09:37:00 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 20EA461C2; Thu, 4 May 2023 09:37:00 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-3.6 required=5.0 tests=ALL_TRUSTED, AWL, NICE_REPLY_A, T_SCC_BODY_TEXT_LINE autolearn=disabled version=3.4.6 X-Spam-Score: -3.6 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id F3EF25F5F; Thu, 4 May 2023 09:36:57 +0200 (CEST) Message-ID: <42b3f9fc-ec61-0370-9a3d-af2101c8e712@lysator.liu.se> Date: Thu, 4 May 2023 09:36:56 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [v2] [RFC] dpaa2: replace system("echo ...") with file i/o To: "Sachin Saxena (OSS)" , dev@dpdk.org, stephen@networkplumber.org References: <20220602214957.150071-1-stephen@networkplumber.org> <20230502095410.487696-1-sachin.saxena@oss.nxp.com> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= In-Reply-To: <20230502095410.487696-1-sachin.saxena@oss.nxp.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2023-05-02 11:54, Sachin Saxena (OSS) wrote: > From: Stephen Hemminger > > Using system() is a bad idea in driver code because it introduces > a number of potential security issues. The codeql analysis tool > flags this a potential security issue. > > Instead just use normal stdio to do the same thing. > > Compile test only, do not have this hardware and therefore can > not test this. > > Signed-off-by: Stephen Hemminger > Reviewed-by: Sachin Saxena > --- > drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 45 ++++++++++++++++-------- > 1 file changed, 31 insertions(+), 14 deletions(-) > > diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c > index 4aec7b2cd8..990cfc5d3b 100644 > --- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c > +++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c > @@ -125,14 +125,21 @@ static void > dpaa2_affine_dpio_intr_to_respective_core(int32_t dpio_id, int cpu_id) > { > #define STRING_LEN 28 > -#define COMMAND_LEN 50 > +#define AFFINITY_LEN 128 > +#define CMD_LEN 300 > uint32_t cpu_mask = 1; > - int ret; > - size_t len = 0; > - char *temp = NULL, *token = NULL; > - char string[STRING_LEN], command[COMMAND_LEN]; > + size_t len = CMD_LEN; > + char *temp, *token = NULL; > + char string[STRING_LEN]; > + char smp_affinity[AFFINITY_LEN]; > FILE *file; > > + temp = (char *)malloc(len * sizeof(char)); No cast is necessary to go from void * to any other pointer. sizeof(char) is by definition one. What you allocate from the heap are in units of chars, so multiplying with sizeof(char) doesn't make sense. How is pre-allocating the buffer an improvement over the old code (deferring memory allocation to getline())? > + if (temp == NULL) { > + DPAA2_BUS_WARN("Unable to allocate temp buffer"); > + return; > + } > + > snprintf(string, STRING_LEN, "dpio.%d", dpio_id); > file = fopen("/proc/interrupts", "r"); > if (!file) { > @@ -155,17 +162,27 @@ dpaa2_affine_dpio_intr_to_respective_core(int32_t dpio_id, int cpu_id) > } > > cpu_mask = cpu_mask << cpu_id; > - snprintf(command, COMMAND_LEN, "echo %X > /proc/irq/%s/smp_affinity", > - cpu_mask, token); > - ret = system(command); > - if (ret < 0) > - DPAA2_BUS_DEBUG( > - "Failed to affine interrupts on respective core"); > - else > - DPAA2_BUS_DEBUG(" %s command is executed", command); > - > + snprintf(smp_affinity, AFFINITY_LEN, > + "/proc/irq/%s/smp_affinity", token); > + /* Free 'temp' memory after using the substring 'token' */ > free(temp); > fclose(file); > + > + file = fopen(smp_affinity, "w"); > + if (file == NULL) { > + DPAA2_BUS_WARN("Failed to open %s", smp_affinity); > + return; > + } > + fprintf(file, "%X\n", cpu_mask); > + fflush(file); > + > + if (ferror(file)) { > + fclose(file); > + DPAA2_BUS_WARN("Failed to write to %s", smp_affinity); > + return; > + } > + > + fclose(file); > } > > static int dpaa2_dpio_intr_init(struct dpaa2_dpio_dev *dpio_dev)