On Thu, May 18, 2023 at 11:00 PM Ilya Maximets <[email protected]> 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>
[email protected] IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
