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

[Andrew Lunn]

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