This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] remote.c: Avoid multiple serial_close calls on baud rateerror


One of my colleagues recently noticed the following:

    (gdb) set remotebaud 0x100000
    (gdb) target remote /dev/ttyS0
    warning: Invalid baud rate 1048576.  Maximum value is 460800.
    /dev/ttyS0: Invalid argument.
    (gdb) set remotebaud 230400
    (gdb) target remote /dev/ttyS0
    Segmentation fault

The reason for this SEGV is that remote.c was closing ``remote_desc''
twice.  On the second attempted close, it was accessing some data
structures through some already freed (and probably even reallocated)
memory.

The comment that I've added explains how the double close is avoided.

FWIW, I considered calling remote_close(), but decided against it
since remote_desc can not be passed explicitly to this function. Also, if the implementation of remote_close() were to change in some
way, it may end up doing more (or less) than what's desired for
handling the baud rate error. Conversely, a hypothetical change in
remote_close() may require that the error handling code be changed in
a similar fashion, so the preferred path to fixing this problem isn't
quite so clear cut. Therefore, I'm willing to revise this patch to
call remote_close() instead if that's deemed preferable.


With regard to the testcase above, it'd be nice if this could be added
to the testsuite, but I can't think of a portable way of doing so.

Calling target_close() here wouldn't be right. The target isn't yet open, the push call only occures further down. This begs the question: why was open called twice? I suspect unpush_target should only call target_close on open/pushed targets.


Anyway, this change is fine. It makes the relevant code more robust.

Andrew



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]