trivial fix for pmemrange
Index: uvm/uvm_pmemrange.c === RCS file: /cvs/src/sys/uvm/uvm_pmemrange.c,v retrieving revision 1.37 diff -p -u -r1.37 uvm_pmemrange.c --- uvm/uvm_pmemrange.c 6 Feb 2014 16:40:40 - 1.37 +++ uvm/uvm_pmemrange.c 13 Feb 2014 22:28:29 - @@ -1647,7 +1647,7 @@ uvm_pmr_rootupdate(struct uvm_pmemrange * Cache the lower page, so we can page-walk later. */ low = root; - low_next = RB_RIGHT(low, objt); + low_next = RB_LEFT(low, objt); while (low_next != NULL PMR_INTERSECTS_WITH( atop(VM_PAGE_TO_PHYS(low_next)), atop(VM_PAGE_TO_PHYS(low_next)) + low_next-fpgsz, @@ -1655,8 +1655,11 @@ uvm_pmr_rootupdate(struct uvm_pmemrange low = low_next; if (uvm_pmr_pg_to_memtype(low) == memtype) return low; - low_next = RB_RIGHT(low, objt); + low_next = RB_LEFT(low, objt); } + + if (low == high) + return NULL; /* * Ack, no hits. Walk the address tree until to find something usable.
Re: trivial fix for pmemrange
the original implementation use identical logic for both ‘high’ ‘low’, which will cause ‘high’ ‘low’ end up at same RB-tree node, instead of an expected interval. and finally, break ‘for’ loop logic luckily all these conditions never meet in practice. On Feb 17, 2014, at 5:14 PM, Mike Larkin mlar...@azathoth.net wrote: Probably get a better response if you explained what this diff does and/or fixes… i just think this bug is a little too obvious
PATCH: fix bug in handling genmask
hope this time i get the part ‘poke claudio@’ right
a. fix a bug.
b. get rid of some junk in ‘mask_rnhead’.
c. forbid unprivileged user to insert ‘genmask' into ‘mask_rnhead'
bug is in this line
memcmp((caddr_t *)info.rti_info[RTAX_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
memcmp((caddr_t *)info.rti_info[RTAX_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.136
diff -u -p -r1.136 rtsock.c
--- net/rtsock.c21 Jan 2014 10:08:02 - 1.136
+++ net/rtsock.c21 Jan 2014 21:27:32 -
@@ -564,20 +564,25 @@ route_output(struct mbuf *m, ...)
error = EINVAL;
goto flush;
}
- if (info.rti_info[RTAX_GENMASK] != NULL) {
- struct radix_node *t;
- t = rn_addmask(info.rti_info[RTAX_GENMASK], 0, 1);
- if (t info.rti_info[RTAX_GENMASK]-sa_len =
- ((struct sockaddr *)t-rn_key)-sa_len
- memcmp((caddr_t *)info.rti_info[RTAX_GENMASK] + 1,
- (caddr_t *)t-rn_key + 1,
- ((struct sockaddr *)t-rn_key)-sa_len) - 1)
- info.rti_info[RTAX_GENMASK] =
- (struct sockaddr *)(t-rn_key);
- else {
+ if (info.rti_info[RTAX_GENMASK] != NULL rtm-rtm_type != RTM_GET) {
+ rnh = rt_gettable(info.rti_info[RTAX_DST]-sa_family, tableid);
+ if (rnh == NULL) {
+ error = EINVAL;
+ goto flush;
+ }
+ rn = rn_addmask(info.rti_info[RTAX_GENMASK], 0,
+ rnh-rnh_treetop-rn_off);
+ if (rn == NULL) {
error = ENOBUFS;
goto flush;
}
+ if (!((struct sockaddr *)rn-rn_key)-sa_len) {
+ error = EINVAL;
+ goto flush;
+ }
+ info.rti_info[RTAX_GENMASK] = (struct sockaddr *)rn-rn_key;
+ } else {
+ info.rti_info[RTAX_GENMASK] = NULL;
}
#ifdef MPLS
info.rti_mpls = rtm-rtm_mpls;
Re: PATCH: fix bug in handling genmask
maybe, one problem at a time?
first, whether there is actually a bug there, this patch fix it?
and then this removing genmask discussion
as i understand, unless you guys had already reached a conclusion once, there
might be no immediate result
and as blambert@ mentioned, whether it is really a bug might matter
Kieran
On Dec 28, 2013, at 5:19 PM, Bret Lambert blamb...@openbsd.org wrote:
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 - 1.131
+++ net/rtsock.c 27 Dec 2013 14:08:44 -
@@ -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;
PATCH: fix bug in handling genmask
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.c1 Nov 2013 20:09:14 - 1.131
+++ net/rtsock.c27 Dec 2013 14:08:44 -
@@ -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;
PATCH: fix bug in handling genmask
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.c1 Nov 2013 20:09:14 - 1.131
+++ net/rtsock.c27 Dec 2013 14:08:44 -
@@ -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;
PATCH: Fix invalid size to 'memcmp' in 'rn_lexobetter'
maybe someone could verify commit this patch
this bug was introduced in r1.28 of ‘src/sys/net/radix.c’
i found it while going through source codes
Index: sys/net/radix.c
===
RCS file: /cvs/src/sys/net/radix.c,v
retrieving revision 1.31
diff -u -p -8 -r1.31 radix.c
--- sys/net/radix.c 20 Oct 2013 16:17:36 - 1.31
+++ sys/net/radix.c 10 Dec 2013 09:13:58 -
@@ -471,34 +471,33 @@ rn_addmask(void *n_arg, int search, int
if (isnormal)
x-rn_flags |= RNF_NORMAL;
return (x);
}
static int /* XXX: arbitrary ordering for non-contiguous masks */
rn_lexobetter(void *m_arg, void *n_arg)
{
- u_char *mp = m_arg, *np = n_arg, *lim;
+ u_char *mp = m_arg, *np = n_arg;
/*
* Longer masks might not really be lexicographically better,
* but longer masks always have precedence since they must be checked
* first. The netmasks were normalized before calling this function and
* don't have unneeded trailing zeros.
*/
if (*mp *np)
return 1;
if (*mp *np)
return 0;
/*
* Must return the first difference between the masks
* to ensure deterministic sorting.
*/
- lim = mp + *mp;
- return (memcmp(mp, np, *lim) 0);
+ return (memcmp(mp, np, *mp) 0);
}
static struct radix_mask *
rn_new_radix_mask(struct radix_node *tt, struct radix_mask *next)
{
struct radix_mask *m;
MKGet(m);
