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

Andrew Lunn
2024-05-13T20:17:59Z

> +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