Bug 27133

Summary: Crash with set logging redirect and debugredirect
Product: gdb Reporter: Nate Eldredge <nate>
Component: gdbAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: lsix, nate, tromey
Priority: P2    
Version: HEAD   
Target Milestone: 11.1   
Host: Target:
Build: Last reconfirmed:
Attachments: Patch to fix the issue

Description Nate Eldredge 2020-12-31 07:08:27 UTC
gdb crashes with SIGABRT when I run the following sequence of commands:

set logging redirect on
set logging debugredirect on
set logging on

Reproduced with latest HEAD (commit 391750c35548611) on x86_64-pc-linux-gnu (Ubuntu 20.04).  Also reproduced with Ubuntu's 9.2.0 on x86_64 Ubuntu 20.04 and aarch64 Ubuntu 20.10 (where it gets SIGBUS instead).
Comment 1 Lancelot SIX 2021-01-13 12:11:42 UTC
Created attachment 13118 [details]
Patch to fix the issue

Hi,

This comes dowm to a use after delete problem.

The attach patch has been proposed to gdb-patches@.

I cannot assign myself to the bug so I mainly attach this patch so no-one spends time investigating on this problem.

Multiple implementations are possible to solve this problem, none entirely satisfactory to my taste.
Comment 2 Sourceware Commits 2021-01-27 22:16:13 UTC
The master branch has been updated by Lancelot SIX <lsix@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=59b59f08f6448a77730c8d8dde5871f1bf6806d0

commit 59b59f08f6448a77730c8d8dde5871f1bf6806d0
Author: Lancelot SIX <lsix@lancelotsix.com>
Date:   Fri Jan 1 20:11:28 2021 +0000

    Avoid use after free with logging and debug redirect.
    
    This patch addresses PR gdb/27133. Before it, the following succession
    of commands would cause gdb to crash:
    
            set logging redirect on
            set logging debugredirect on
            set logging on
    
    The problem eventually comes down to a use after free. The function
    cli_interp_base::set_logging is called with a unique_ptr argument that
    holds a pointer to the redirection file. In the problematic use case,
    no-one ever took ownership of that pointer (as far as unique_ptr is
    concerned), so the call to its dtor at the end of the function causes
    the file object to be deleted. Any later use of the pointer to the
    redirection file is therefore an error.
    
    This patch ensures that the unique_ptr is released  when required (so it
    does not assume ownership anymore). The internal logic of
    cli_interp_base::set_logging takes care of freeing the ui_file when it
    is not necessary anymore using the saved_output.file_to_delete field.
    
    gdb/ChangeLog:
    
            PR gdb/27133
            * cli/cli-interp.c (cli_interp_base::set_logging): Ensure the
            unique_ptr is released when the wrapped pointer is kept for later
            use.
    
    gdb/testsuite/ChangeLog:
    
            PR gdb/27133
            * gdb.base/ui-redirect.exp: Add test case that ensures that
            redirecting both logging and debug does not cause gdb to crash.
Comment 3 Tom Tromey 2021-01-28 13:56:30 UTC
Fixed.