(resend, seems to have got lost)
Begin forwarded message:
Date: Fri, 8 Jul 2005 21:42:21 +1000
From: Paul Mackerras <[EMAIL PROTECTED]>
To: Jeff Garzik <[EMAIL PROTECTED]>, [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED], [EMAIL PROTECTED], netdev@vger.kernel.org, [EMAIL
PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED], [EMAIL PROTECTED]
Subject: Re: [Fwd: [patch 02/15] ppp_mppe: add PPP MPPE encryption module]
Some comments on the MPPE kernel patch (sorry it's taken me so long):
> +static inline struct sk_buff *
> +pad_compress_skb(struct ppp *ppp, struct sk_buff *skb)
> +{
> + struct sk_buff *new_skb;
> + int len;
> + int new_skb_size = ppp->dev->mtu + ppp->xcomp->comp_skb_extra_space +
> ppp->dev->hard_header_len;
> + int compressor_skb_size = ppp->dev->mtu +
> ppp->xcomp->comp_skb_extra_space + PPP_HDRLEN;
This would be nicer if you broke these lines each into two lines. It
would help if you chose shorter names for comp_skb_extra_space and
decomp_skb_extra_space (just comp_extra, for instance). And why do we
need decomp_skb_extra_space anyway? We expect the decompressor to
expand things; if they shrink (as they will with MPPE) we don't have
to do anything special.
> + } else {
> + /*
> + * (len < 0)
> + * MPPE requires that we do not send unencrypted
> + * frames. The compressor will return -1 if we
> + * should drop the frame. We cannot simply test
> + * the compress_proto because MPPE and MPPC share
> + * the same number.
> + */
> + if (net_ratelimit())
> + printk(KERN_ERR "ppp: compressor dropped pkt\n");
> + kfree_skb(new_skb);
> + new_skb = NULL;
I think we just leaked the original skb in this case.
> @@ -1683,7 +1716,7 @@ ppp_decompress_frame(struct ppp *ppp, st
> goto err;
>
> if (proto == PPP_COMP) {
> - ns = dev_alloc_skb(ppp->mru + PPP_HDRLEN);
> + ns = dev_alloc_skb(ppp->mru +
> ppp->rcomp->decomp_skb_extra_space + PPP_HDRLEN);
> if (ns == 0) {
> printk(KERN_ERR "ppp_decompress_frame: no memory\n");
> goto err;
I don't see where you make sure that you drop any unencrypted received
frames.
> diff -puN /dev/null drivers/net/ppp_mppe.c
I haven't looked at this in detail, presumably it works OK.
Paul.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
