netdev - Re: [PATCH 1/3] mctp pcc: Implement MCTP over PCC Transport

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: <48ef7972-4b8f-4027-a2ca-357e53dcdd0f@lunn.ch>
Date: Mon, 13 May 2024 22:17:48 +0200
From: Andrew Lunn <andrew@...n.ch>
To: admiyo@...amperecomputing.com
Cc: Jeremy Kerr <jk@...econstruct.com.au>,
	Matt Johnston <matt@...econstruct.com.au>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] mctp pcc: Implement MCTP over PCC Transport

> +static struct mctp_pcc_packet *mctp_pcc_extract_data(struct sk_buff *old_skb,
> +						     void *buffer, int outbox_index)
> +{
> +	struct mctp_pcc_packet *mpp;
> +
> +	mpp = buffer;
> +	writel(PCC_MAGIC | outbox_index, &mpp->pcc_header.signature);
> +	writel(0x1, &mpp->pcc_header.flags);
> +	memcpy_toio(mpp->pcc_header.mctp_signature, MCTP_SIGNATURE, SIGNATURE_LENGTH);
> +	writel(old_skb->len + SIGNATURE_LENGTH,  &mpp->pcc_header.length);
> +	memcpy_toio(mpp->header_data,    old_skb->data, old_skb->len);
> +	return mpp;
> +}

...

> +static netdev_tx_t mctp_pcc_tx(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	unsigned char *buffer;
> +	struct mctp_pcc_ndev *mpnd;
> +	struct mctp_pcc_packet  *mpp;
> +	unsigned long flags;
> +	int rc;
> +
> +	netif_stop_queue(ndev);
> +	ndev->stats.tx_bytes += skb->len;
> +	mpnd = (struct mctp_pcc_ndev *)netdev_priv(ndev);
> +	spin_lock_irqsave(&mpnd->lock, flags);
> +	buffer =  mpnd->pcc_comm_outbox_addr;
> +	mpp = mctp_pcc_extract_data(skb, mpnd->pcc_comm_outbox_addr, mpnd->hw_addr.outbox_index);

I don't see any length checks here. How do you know the skb contains
sizeof(struct mctp_pcc_packet)?

> +static int create_mctp_pcc_netdev(struct acpi_device *acpi_dev,
> +				  struct device *dev, int inbox_index,
> +				  int outbox_index)
> +{
> +	int rc;
> +	int mctp_pcc_mtu;
> +	char name[32];
> +	struct net_device *ndev;
> +	struct mctp_pcc_ndev *mctp_pcc_dev;
> +	struct mctp_pcc_hw_addr physical_link_addr;

Since this is networking code, you should be using reverse christmas
tree for all your functions.

> +	snprintf(name, sizeof(name), "mctpipcc%x", inbox_index);
> +	ndev = alloc_netdev(sizeof(struct mctp_pcc_ndev), name, NET_NAME_ENUM, mctp_pcc_setup);

%x is very unusual for network device names.

	Andrew