netdev - Re: [PATCH v4 resend 04/23] ice: Register auxiliary device to provide RDMA

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210407174521.GA542400@nvidia.com>
Date:   Wed, 7 Apr 2021 14:45:21 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Shiraz Saleem <shiraz.saleem@...el.com>
Cc:     dledford@...hat.com, kuba@...nel.org, davem@...emloft.net,
        linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
        david.m.ertman@...el.com, anthony.l.nguyen@...el.com
Subject: Re: [PATCH v4 resend 04/23] ice: Register auxiliary device to
 provide RDMA

On Tue, Apr 06, 2021 at 07:14:43PM -0500, Shiraz Saleem wrote:
> +/**
> + * ice_plug_aux_devs - allocate and register one AUX dev per cdev_info in PF
> + * @pf: pointer to PF struct
> + */
> +int ice_plug_aux_devs(struct ice_pf *pf)
> +{
> +	struct iidc_auxiliary_dev *iadev;
> +	int ret, i;
> +
> +	if (!pf->cdev_infos)
> +		return 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(ice_cdev_ids); i++) {
> +		struct iidc_core_dev_info *cdev_info;
> +		struct auxiliary_device *adev;
> +
> +		cdev_info = pf->cdev_infos[i];
> +		if (!cdev_info)
> +			continue;
> +
> +		iadev = kzalloc(sizeof(*iadev), GFP_KERNEL);
> +		if (!iadev)
> +			return -ENOMEM;
> +
> +		adev = &iadev->adev;
> +		cdev_info->adev = adev;
> +		iadev->cdev_info = cdev_info;
> +
> +		if (ice_cdev_ids[i].id == IIDC_RDMA_ID) {
> +			if (cdev_info->rdma_protocol ==
> +			    IIDC_RDMA_PROTOCOL_IWARP)
> +				adev->name = kasprintf(GFP_KERNEL, "%s_%s",
> +						       ice_cdev_ids[i].name,
> +						       "iwarp");
> +			else
> +				adev->name = kasprintf(GFP_KERNEL, "%s_%s",
> +						       ice_cdev_ids[i].name,
> +						       "roce");
> +		} else {
> +			adev->name = kasprintf(GFP_KERNEL, "%s",
> +					       ice_cdev_ids[i].name);

This never happens, it is dead code, right?

> +		}

This is all confused, the adev->name is supposed to be a compile time
constant, this not be allocating memory and combining the constant
"intel_rdma" and the trailing "iwarp" "roce"

Just use the proper define right here.

Also all these kasprintfs should have their errors checked, which is
also why it shouldn't be written like this.

Jason