IIRC, I'd talked to both claudio and sthen about removing genmask,
as it's unused and slightly nonsensical. Maybe we should just do
it, if there are actual bugs lurking therein?
On Sat, Dec 28, 2013 at 03:57:04AM +0800, Kieran Devlin wrote:
> re-send this patch, loop in claudio@
>
> maybe someone could verify & commit this patch
>
> a. fix a bug
> b. get rid of junk in ?mask_rnhead'
> c. forbid unprivileged user to insert mask into ?mask_rnhead'
>
> bug is in this line
> Bcmp((caddr_t *)genmask + 1, (caddr_t *)t->rn_key + 1, ((struct
> sockaddr *)t->rn_key)->sa_len)
> to make this right, at least, it should look like this
> Bcmp((caddr_t)genmask + 1, (caddr_t)t->rn_key + 1, ((struct sockaddr
> *)t->rn_key)->sa_len - 1)
> after doing this, the whole checking seems completely unnecessary, is
> expected result from ?rn_addmask?.
>
> Index: net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.131
> diff -p -u -r1.131 rtsock.c
> --- net/rtsock.c 1 Nov 2013 20:09:14 -0000 1.131
> +++ net/rtsock.c 27 Dec 2013 14:08:44 -0000
> @@ -593,18 +593,22 @@ route_output(struct mbuf *m, ...)
> error = EINVAL;
> goto flush;
> }
> - if (genmask) {
> - struct radix_node *t;
> - t = rn_addmask(genmask, 0, 1);
> - if (t && genmask->sa_len >=
> - ((struct sockaddr *)t->rn_key)->sa_len &&
> - Bcmp((caddr_t *)genmask + 1, (caddr_t *)t->rn_key + 1,
> - ((struct sockaddr *)t->rn_key)->sa_len) - 1)
> - genmask = (struct sockaddr *)(t->rn_key);
> - else {
> + if (genmask && rtm->rtm_type != RTM_GET) {
> + if (!(rnh = rt_gettable(dst->sa_family, tableid))) {
> + error = EINVAL;
> + goto flush;
> + }
> + if (!(rn = rn_addmask(genmask, 0, rnh->rnh_treetop->rn_off))) {
> error = ENOBUFS;
> goto flush;
> }
> + if (!((struct sockaddr *)rn->rn_key)->sa_len) {
> + error = EINVAL;
> + goto flush;
> + }
> + genmask = (struct sockaddr *)rn->rn_key;
> + } else {
> + genmask = NULL;
> }
> #ifdef MPLS
> info.rti_mpls = rtm->rtm_mpls;
>
