| Message ID | ZNOsq6Z7t/eyIG/9@p100 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix interval_tree_iter_first() to check root node value | expand |
On 8/9/23 08:11, Helge Deller wrote: > Fix a crash in qemu-user when running > > cat /proc/self/maps > > in a chroot, where /proc isn't mounted. > > The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap: > Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the > function read_self_maps() is called and which returns NULL if it can't > read the hosts /proc/self/maps file. Afterwards that NULL is fed into > interval_tree_iter_first() which doesn't check if the root node is NULL. > > Fix it by adding a check if root is NULL and return NULL in that case. > > Signed-off-by: Helge Deller <deller@gmx.de> > Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h") > > diff --git a/util/interval-tree.c b/util/interval-tree.c > index f2866aa7d3..53465182e6 100644 > --- a/util/interval-tree.c > +++ b/util/interval-tree.c > @@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot *root, > { > IntervalTreeNode *node, *leftmost; > > - if (!root->rb_root.rb_node) { > + if (!root || !root->rb_root.rb_node) { I guess this is good enough for 8.1. Before the conversion to interval-tree we would also emit nothing. I've already done a rewrite for 8.2, and I noticed this problem. There I emit what mapping information that I have, which is everything except for the device+path data. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 8/9/23 17:23, Richard Henderson wrote: > On 8/9/23 08:11, Helge Deller wrote: >> Fix a crash in qemu-user when running >> >> cat /proc/self/maps >> >> in a chroot, where /proc isn't mounted. >> >> The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap: >> Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the >> function read_self_maps() is called and which returns NULL if it can't >> read the hosts /proc/self/maps file. Afterwards that NULL is fed into >> interval_tree_iter_first() which doesn't check if the root node is NULL. >> >> Fix it by adding a check if root is NULL and return NULL in that case. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h") >> >> diff --git a/util/interval-tree.c b/util/interval-tree.c >> index f2866aa7d3..53465182e6 100644 >> --- a/util/interval-tree.c >> +++ b/util/interval-tree.c >> @@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot *root, >> { >> IntervalTreeNode *node, *leftmost; >> >> - if (!root->rb_root.rb_node) { >> + if (!root || !root->rb_root.rb_node) { > > > I guess this is good enough for 8.1. Before the conversion to interval-tree we would also emit nothing. Yes and yes. > I've already done a rewrite for 8.2, and I noticed this problem. > There I emit what mapping information that I have, which is > everything except for the device+path data. nice. > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Shall I send a pull request? If so, is it OK that I include this patch in the pull-request as well? linux-user: Fix openat() emulation to correctly detect accesses to /proc https://lists.nongnu.org/archive/html/qemu-devel/2023-08/msg00165.html which already has been R-b: Daniel P. Berrangé Helge
On 8/9/23 08:53, Helge Deller wrote: > On 8/9/23 17:23, Richard Henderson wrote: >> On 8/9/23 08:11, Helge Deller wrote: >>> Fix a crash in qemu-user when running >>> >>> cat /proc/self/maps >>> >>> in a chroot, where /proc isn't mounted. >>> >>> The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap: >>> Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the >>> function read_self_maps() is called and which returns NULL if it can't >>> read the hosts /proc/self/maps file. Afterwards that NULL is fed into >>> interval_tree_iter_first() which doesn't check if the root node is NULL. >>> >>> Fix it by adding a check if root is NULL and return NULL in that case. >>> >>> Signed-off-by: Helge Deller <deller@gmx.de> >>> Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h") >>> >>> diff --git a/util/interval-tree.c b/util/interval-tree.c >>> index f2866aa7d3..53465182e6 100644 >>> --- a/util/interval-tree.c >>> +++ b/util/interval-tree.c >>> @@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot *root, >>> { >>> IntervalTreeNode *node, *leftmost; >>> >>> - if (!root->rb_root.rb_node) { >>> + if (!root || !root->rb_root.rb_node) { >> >> >> I guess this is good enough for 8.1. Before the conversion to interval-tree we would >> also emit nothing. > > Yes and yes. > >> I've already done a rewrite for 8.2, and I noticed this problem. >> There I emit what mapping information that I have, which is >> everything except for the device+path data. > > nice. > >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > Shall I send a pull request? > If so, is it OK that I include this patch in the pull-request as well? > linux-user: Fix openat() emulation to correctly detect accesses to /proc > https://lists.nongnu.org/archive/html/qemu-devel/2023-08/msg00165.html > which already has been R-b: Daniel P. Berrangé I can pick them both up -- I have other linux-user patches to send. r~
diff --git a/util/interval-tree.c b/util/interval-tree.c index f2866aa7d3..53465182e6 100644 --- a/util/interval-tree.c +++ b/util/interval-tree.c @@ -797,7 +797,7 @@ IntervalTreeNode *interval_tree_iter_first(IntervalTreeRoot *root, { IntervalTreeNode *node, *leftmost; - if (!root->rb_root.rb_node) { + if (!root || !root->rb_root.rb_node) { return NULL; }
Fix a crash in qemu-user when running cat /proc/self/maps in a chroot, where /proc isn't mounted. The problem was introduced by commit 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h") where in open_self_maps_1() the function read_self_maps() is called and which returns NULL if it can't read the hosts /proc/self/maps file. Afterwards that NULL is fed into interval_tree_iter_first() which doesn't check if the root node is NULL. Fix it by adding a check if root is NULL and return NULL in that case. Signed-off-by: Helge Deller <deller@gmx.de> Fixes: 3ce3dd8ca965 ("util/selfmap: Rewrite using qemu/interval-tree.h")