Thx for the review, I'll adopt the comment~ On Mon, Jul 20, 2015 at 9:47 AM, Ben Pfaff <b...@nicira.com> wrote:
> On Mon, Jul 20, 2015 at 08:50:39AM -0700, Alex Wang wrote: > > On Mon, Jul 20, 2015 at 8:17 AM, Ben Pfaff <b...@nicira.com> wrote: > > > > > On Mon, Jul 20, 2015 at 01:22:32AM -0700, Alex Wang wrote: > > > > Commit 6fd6ed7 (ofpbuf: Simplify ofpbuf API.) introduced the > > > > 'header' and 'msg' pointers to 'struct ofpbuf'. However, we > > > > forget to update the 'msg' pointer when resizing ofpbuf. > > > > > > > > This bug could cause serious issue. For example, in the function > > > > ofputil_encode_nx_packet_in(), the 'msg' pointer is populated in > > > > ofpraw_alloc_xid() when creating the ofpbuf . Later, the ofpbuf > > > > memory can be reallocated due to the writing to the ofpbuf. > > > > However, since the 'msg' pointer is not updated, the later use of > > > > the 'ofpbuf->msg' will end up writing to either free'ed memory or > > > > memory allocated for other struct. > > > > > > > > This commit fixes the bug by always updating the 'header' and > > > > 'msg' pointers when the ofpbuf is resized. Also, a simple test > > > > is added. > > > > > > > > Signed-off-by: Alex Wang <al...@nicira.com> > > > > > > Good catch! > > > > > > I don't understand the new comment on ofpbuf_trim(). ofpbuf_resize__() > > > will adjust the pointers automatically, won't it? > > > > > > > There is one corner case I tested, assume we 'b = ofpbuf_new(100)' first, > > and then make 'b->header = b->base + 10', 'b->msg = b->base + 50', > without > > actually putting anything to the buffer yet. > > > > Then, calling 'ofpbuf_trim(b)' will trim 'b' to an empty ofpbuf but with > > 'b->header' and 'b->msg' pointing invalid memory addresses. Dont think I > > can find any real implication. What do youthink? > > That case makes sense. Maybe the comment could be more specific: > > * Updates 'b->header' and 'b->msg' so that they point to the same > locations in > * the data. (If they pointed into the tailroom or headroom then they > become > * invalid.) > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
