On Thu, May 18, 2023 at 11:00 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
> On 5/18/23 17:53, Ilya Maximets wrote: > > On 3/23/23 16:09, Ales Musil wrote: > >> +log_received_backtrace(int fd) { > > > > '{' should be on a separate line. > > > >> + struct backtrace bt; > >> + > >> + if (read_received_backtrace(fd, &bt, sizeof bt)) { > >> + struct ds ds = DS_EMPTY_INITIALIZER; > > > > An empty line here. > > > >> + ds_put_cstr(&ds, BACKTRACE_DUMP_MSG); > >> + backtrace_format(&bt, &ds); > > > > read_received_backtrace() only varifies that it received more than > > zero bytes. But we should, probably, verify that at least n_frames > > is valid. Otherwise, we may crash the monitor trying to resolve > > symbols and print them out. We may also need to adjust the n_frames > > to the number of actually received frames if the dying process > > didn't manage to write all of them. > > Scratch that. We're zeroing out the memory before receiving the data. > Assuming backtrace_symbol is capable of handling NULL pointers, we > should be fine. It might still make sense to check that n_frames is > below the maximum and cap it at maximum value if it's not. For the > case it got corrupted before being sent. > > Best regards, Ilya Maximets. > > Hi Ilya, thank you for the review, everything should be fixed in v4. Regards, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> amu...@redhat.com IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev