| Message ID | 20240307150151.665230-1-amusil@redhat.com |
|---|---|
| State | Accepted |
| Commit | 5339ce386f3cccc4972bc49c3f68272138d58511 |
| Headers | show |
| Series | [ovs-dev] ofpbuf: Prevent undefined behavior in ofpbuf_clone | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | fail | test: fail |
On 3/7/24 16:01, Ales Musil wrote: > The new_buffer data pointer is NULL when the size of the cloned > buffer is 0. This is fine as there is no need to allocate space. > However, the cloned buffer header/msg might be the same pointer > as data. This causes undefined behavior by adding 0 to NULL pointer. > Check if the data buffer is not NULL before attempting to apply the > header/msg offset. > > This was caught by OVN system test: > > lib/ofpbuf.c:203:56: runtime error: applying zero offset to null pointer > #0 0xa012fc in ofpbuf_clone_with_headroom /workspace/ovn/ovs/lib/ofpbuf.c:203:56 > #1 0x635fd4 in put_remote_port_redirect_overlay /workspace/ovn/controller/physical.c:397:40 > #2 0x635fd4 in consider_port_binding /workspace/ovn/controller/physical.c:1951:9 > #3 0x62e046 in physical_run /workspace/ovn/controller/physical.c:2447:9 > #4 0x601d98 in en_pflow_output_run /workspace/ovn/controller/ovn-controller.c:4690:5 > #5 0x707769 in engine_recompute /workspace/ovn/lib/inc-proc-eng.c:415:5 > #6 0x7060eb in engine_compute /workspace/ovn/lib/inc-proc-eng.c:454:17 > #7 0x7060eb in engine_run_node /workspace/ovn/lib/inc-proc-eng.c:503:14 > #8 0x7060eb in engine_run /workspace/ovn/lib/inc-proc-eng.c:528:9 > #9 0x5f9f26 in main /workspace/ovn/controller/ovn-controller.c > > Signed-off-by: Ales Musil <amusil@redhat.com> > --- Thanks! Applied and backported down to 2.17. Best regards, Ilya Maximets.
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c index d3d42b414..232ebeb97 100644 --- a/lib/ofpbuf.c +++ b/lib/ofpbuf.c @@ -197,12 +197,12 @@ ofpbuf_clone_with_headroom(const struct ofpbuf *b, size_t headroom) struct ofpbuf *new_buffer; new_buffer = ofpbuf_clone_data_with_headroom(b->data, b->size, headroom); - if (b->header) { + if (new_buffer->data && b->header) { ptrdiff_t header_offset = (char *) b->header - (char *) b->data; new_buffer->header = (char *) new_buffer->data + header_offset; } - if (b->msg) { + if (new_buffer->data && b->msg) { ptrdiff_t msg_offset = (char *) b->msg - (char *) b->data; new_buffer->msg = (char *) new_buffer->data + msg_offset;
The new_buffer data pointer is NULL when the size of the cloned buffer is 0. This is fine as there is no need to allocate space. However, the cloned buffer header/msg might be the same pointer as data. This causes undefined behavior by adding 0 to NULL pointer. Check if the data buffer is not NULL before attempting to apply the header/msg offset. This was caught by OVN system test: lib/ofpbuf.c:203:56: runtime error: applying zero offset to null pointer #0 0xa012fc in ofpbuf_clone_with_headroom /workspace/ovn/ovs/lib/ofpbuf.c:203:56 #1 0x635fd4 in put_remote_port_redirect_overlay /workspace/ovn/controller/physical.c:397:40 #2 0x635fd4 in consider_port_binding /workspace/ovn/controller/physical.c:1951:9 #3 0x62e046 in physical_run /workspace/ovn/controller/physical.c:2447:9 #4 0x601d98 in en_pflow_output_run /workspace/ovn/controller/ovn-controller.c:4690:5 #5 0x707769 in engine_recompute /workspace/ovn/lib/inc-proc-eng.c:415:5 #6 0x7060eb in engine_compute /workspace/ovn/lib/inc-proc-eng.c:454:17 #7 0x7060eb in engine_run_node /workspace/ovn/lib/inc-proc-eng.c:503:14 #8 0x7060eb in engine_run /workspace/ovn/lib/inc-proc-eng.c:528:9 #9 0x5f9f26 in main /workspace/ovn/controller/ovn-controller.c Signed-off-by: Ales Musil <amusil@redhat.com> --- lib/ofpbuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)