Received: by 2002:a05:6359:6284:b0:131:369:b2a3 with SMTP id se4csp3007176rwb; Mon, 7 Aug 2023 07:00:52 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFkFD/8Ui9x5drYQOH1heeR9gyrxNRzKmYWGfwpJXp28PXC15EiksT3bcIKS6eqzl0Ldd/F X-Received: by 2002:a05:6a20:1008:b0:13f:7c0e:dc6c with SMTP id gs8-20020a056a20100800b0013f7c0edc6cmr7760349pzc.38.1691416852091; Mon, 07 Aug 2023 07:00:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691416852; cv=none; d=google.com; s=arc-20160816; b=pQMXhRfge1jAaNoZ8/QIBVk5nlMYBj3ivn0mZ5g5X01M78CAKl9rO45iJzSwz2azub O7lRNU9rD3W6Vb+b5FWUuWQVvZVrfoIB9c2KGInA4QLj5GMgHrYq+Z2UqwiiZk/I1eP2 Tt+CVCIDwApufNtF0+C2FCLHmzODAxWp52eELj5v29T7ja2P6xPg0FAqKjeoL5OeBIAj PUs8qzBr4LoReriqKDkAdjEwtm3W5Y3quMK1U4Ym5KXp/awhbv3SiekSlvCkU8JBdvsx tLuLmrj03CEJzEo3B7KycuLKkIhkfY9LtSYzrQD2e6YbVWiHSZUK0mhem0m0r1Y+1j+N yvOw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=Q4j/unLw5xI0DQhKe0uiyRNqI2uCtP+kijLLs4Ra99g=; fh=pGXrYiI8+Crd0eXAy9chDNiq7Zj1NGE3camNdxC3zmE=; b=n/yD6Zkn0F/xBnGmHeMPeDIXPtITMbVrmMHF2NJxhWgy1PjlvzMepIOXwmcsntMlHW E7s40Ty1xFQtMBmbZ4LNV0qFh9f/8UnIyg/Vdud682n/7YFiQsggiZBSsTUIHMAIUckY +HcRTytkGfwzqjoY1zirt7r4MKwM2msOBJmnuMu1OxA5DwjzLZJCuI/1QlqBCHtY82bS mWLyNzdO9IUL0BAUaVe3QKnr2AJc5unOVqTYpJNLlLAczdEDXWGwRty4zIWbAERvHNLX uZqrgWalkymcNmDocYUQLZ1F2YXq94OA3IcC7du+ogZg+1u+NDATd6b1KqS+T8vQssn1 M4Ow== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=FihCNa5S; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y187-20020a638ac4000000b00564514df65bsi5621975pgd.890.2023.08.07.07.00.30; Mon, 07 Aug 2023 07:00:52 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=FihCNa5S; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233672AbjHGNtC (ORCPT + 99 others); Mon, 7 Aug 2023 09:49:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36746 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234433AbjHGNs5 (ORCPT ); Mon, 7 Aug 2023 09:48:57 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6B49610DE for ; Mon, 7 Aug 2023 06:48:53 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9B89A61C0D for ; Mon, 7 Aug 2023 13:48:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 48459C433C8; Mon, 7 Aug 2023 13:48:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1691416132; bh=wuj6SUxGrJEiEaxycf3zFEQQB3mPHY/qFqe5ETkwJiU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=FihCNa5SWcVtfcpLwVD5yaeGTxf3a2H0Wxykjq3H5CxSG+3RTbpwp0bMSRt1Jz2fA 0zAJTqQpEO6ELWTg6mi5btIMhf3zP6gJbvqL3jAHd4tFmWrLA9e/nv9RK9Z0Jwvdqz E88Pff9W49oRe+thFv+Lp0Xt/IlsiXwv3tEtcNVzKpNUQZAsImbFVQLZsP+W3v9s41 AgZy0oZPqzFcJVRKYC/wEwcGG+MALGU/3TOBamHq896+uv/KNk80zW35n/45grsfYN YdIp4P0zoCIfRTkZdoolRpyirrVJ+Q2tAHVxIknan7BYHex3oDfCeSytEj1p+ftCK3 nGQekgO61edkg== Date: Mon, 7 Aug 2023 15:48:46 +0200 From: Simon Horman To: Matthew Cover Cc: Michael Chan , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Richard Cochran , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Matthew Cover , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org Subject: Re: [PATCH net-next] Add bnxt_netlink to facilitate representor pair configurations. Message-ID: References: <20230804212954.98868-1-matthew.cover@stackpath.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230804212954.98868-1-matthew.cover@stackpath.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 04, 2023 at 02:29:54PM -0700, Matthew Cover wrote: ... Hi Matthew, > +static int bnxt_parse_attrs(struct nlattr **a, struct bnxt **bp, > + struct net_device **dev) > +{ > + pid_t pid; > + struct net *ns = NULL; > + const char *drivername; > + > + if (!a[BNXT_ATTR_PID]) { > + netdev_err(*dev, "No process ID specified\n"); > + goto err_inval; > + } > + pid = nla_get_u32(a[BNXT_ATTR_PID]); > + ns = get_net_ns_by_pid(pid); > + if (IS_ERR(ns)) { > + netdev_err(*dev, "Invalid net namespace for pid %d (err: %ld)\n", > + pid, PTR_ERR(ns)); > + goto err_inval; > + } > + > + if (!a[BNXT_ATTR_IF_INDEX]) { > + netdev_err(*dev, "No interface index specified\n"); > + goto err_inval; > + } > + *dev = dev_get_by_index(ns, nla_get_u32(a[BNXT_ATTR_IF_INDEX])); > + > + put_net(ns); > + ns = NULL; > + if (!*dev) { > + netdev_err(*dev, "Invalid network interface index %d (err: %ld)\n", > + nla_get_u32(a[BNXT_ATTR_IF_INDEX]), PTR_ERR(ns)); > + goto err_inval; > + } > + if (!(*dev)->dev.parent || !(*dev)->dev.parent->driver || > + !(*dev)->dev.parent->driver->name) { > + netdev_err(*dev, "Unable to get driver name for device %s\n", > + (*dev)->name); > + goto err_inval; > + } > + drivername = (*dev)->dev.parent->driver->name; > + if (strcmp(drivername, DRV_MODULE_NAME)) { > + netdev_err(*dev, "Device %s (%s) is not a %s device!\n", > + (*dev)->name, drivername, DRV_MODULE_NAME); > + goto err_inval; > + } > + *bp = netdev_priv(*dev); > + if (!*bp) { We only get here if *bp is NULL. But on the following line *bp is dereferenced. Perhaps this should be netdev_warn(*dev, ...) Flagged by Smatch. > + netdev_warn((*bp)->dev, "No private data\n"); > + goto err_inval; > + } > + > + return 0; > + > +err_inval: > + if (ns && !IS_ERR(ns)) > + put_net(ns); > + return -EINVAL; > +} > + > +/* handler */ > +static int bnxt_netlink_hwrm(struct sk_buff *skb, struct genl_info *info) > +{ > + struct nlattr **a = info->attrs; > + struct net_device *dev = NULL; > + struct sk_buff *reply = NULL; > + struct input *req, *nl_req; > + struct bnxt *bp = NULL; > + struct output *resp; > + int len, rc; > + void *hdr; > + > + rc = bnxt_parse_attrs(a, &bp, &dev); > + if (rc) > + goto err_rc; > + > + if (!bp) { > + rc = -EINVAL; > + goto err_rc; > + } > + > + if (!bp->hwrm_dma_pool) { > + netdev_warn(bp->dev, "HWRM interface not currently available on %s\n", > + dev->name); > + rc = -EINVAL; > + goto err_rc; > + } > + > + if (!a[BNXT_ATTR_REQUEST]) { > + netdev_warn(bp->dev, "No request specified\n"); > + rc = -EINVAL; > + goto err_rc; > + } > + len = nla_len(a[BNXT_ATTR_REQUEST]); > + nl_req = nla_data(a[BNXT_ATTR_REQUEST]); > + > + reply = genlmsg_new(PAGE_SIZE, GFP_KERNEL); > + if (!reply) { > + netdev_warn(bp->dev, "Error: genlmsg_new failed\n"); > + rc = -ENOMEM; > + goto err_rc; > + } > + > + rc = hwrm_req_init(bp, req, nl_req->req_type); hwrm_req_init() expects a variable of type u16 as it's type parameter. But the tupe of nl_req->req_type is __le32. As flagged by Sparse. > + if (rc) > + goto err_rc; > + > + rc = hwrm_req_replace(bp, req, nl_req, len); > + if (rc) > + goto err_rc; > + > + resp = hwrm_req_hold(bp, req); > + rc = hwrm_req_send_silent(bp, req); > + if (rc) { > + /* > + * Indicate success for the hwrm transport, while letting > + * the hwrm error be passed back to the netlink caller in > + * the response message. Caller is responsible for handling > + * any errors. > + * > + * no kernel warnings are logged in this case. > + */ > + rc = 0; > + } > + hdr = genlmsg_put_reply(reply, info, &bnxt_netlink_family, 0, > + BNXT_CMD_HWRM); > + if (nla_put(reply, BNXT_ATTR_RESPONSE, resp->resp_len, resp)) { Likewise, the type of resp->resp_len is __le16, but an int is expected. > + netdev_warn(bp->dev, "No space for response attribte\n"); > + hwrm_req_drop(bp, req); > + rc = -ENOMEM; > + goto err_rc; > + } > + genlmsg_end(reply, hdr); > + hwrm_req_drop(bp, req); > + > + dev_put(dev); > + dev = NULL; > + > + return genlmsg_reply(reply, info); > + > +err_rc: > + if (reply && !IS_ERR(reply)) > + kfree_skb(reply); > + if (dev && !IS_ERR(dev)) > + dev_put(dev); > + > + if (bp) > + netdev_warn(bp->dev, "returning with error code %d\n", rc); > + > + return rc; > +} ...