From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id D63BE3858D35 for ; Tue, 14 Feb 2023 15:52:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D63BE3858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676389975; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=nimENSKaiS7ga8LHg7xF3WjcKplz0+PU+7HSgIO0sDc=; b=G6cUKWUt5KUfckkKEJhjXwxectd8gfeR23uVrUKcoSGQIPaAEP832xKGg2UGTL/FKpIUFQ rhIC3U2zwXjN7H4DEYy2Uj8mtRX+j641vdCUOONiTQr8lNC1QyNKg9LAgaqnJpwr0JPeO5 kLLmypvSREzQ4UQhQ5MMJdM4iAA+4sA= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-211-t7wIK3QRPH2S_LYN4k1g4g-1; Tue, 14 Feb 2023 10:52:54 -0500 X-MC-Unique: t7wIK3QRPH2S_LYN4k1g4g-1 Received: by mail-qk1-f199.google.com with SMTP id h13-20020a05620a244d00b006fb713618b8so9652761qkn.0 for ; Tue, 14 Feb 2023 07:52:54 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=nimENSKaiS7ga8LHg7xF3WjcKplz0+PU+7HSgIO0sDc=; b=wHyzC1kte4RdjaWgwS70gCt4jv/pAwHQ/E/SOafRzmIwA4QBR0UORoAvjz63PNZDtd W3wNazwejjGMWxPaoVsNyNBrfh+xbB+TYin0w5Bfoy8ffFAsayaTb0+iwQJ0D6GXbbOf IqLwgDFy5feorAVpVodg4tBUyq8wqealqPAQIwOUiyQ0jG9dQ9WLCRhqll7XGSUjLMgF sNjQuqABZHmUrsfHmvnIG5gF1yXGLQG6qHjkUl+JYKfEPCktmtHrDAz37yf57wDhMvBv NpxMsrOaIGnEivNKcXjPop7aj/pO2towQ+8ZO8+jNyaluBb8iuBOltX7EmeN57soWfho 9HZQ== X-Gm-Message-State: AO0yUKVjwuIe3uDLg+mBoKH0a33gyu4R88mRa2NB1d3tf1dBMroNwhRS jBfYQMcN7zaNK/TGe/cj4aWgWIRNZ2ttOaApqgOUB+pxwBGhHc5IrKEQNE/VMfafIsGxq3KVGCM OWGQko0TUNAHGhWXD6Ve1RdEiQ96s9t7EVuhJFOkPvndYIXfatw0Aqgo81gjWQW/9Y9nQjSTARJ pkvtc= X-Received: by 2002:ac8:5902:0:b0:3ba:19e5:3e45 with SMTP id 2-20020ac85902000000b003ba19e53e45mr4439235qty.13.1676389973806; Tue, 14 Feb 2023 07:52:53 -0800 (PST) X-Google-Smtp-Source: AK7set91S89RnbUcJiBkLX6clqHH6EkLeNUFC32H/zIolkP+49Br2UCl7864qrMQZQLvSk8LoCnhAw== X-Received: by 2002:ac8:5902:0:b0:3ba:19e5:3e45 with SMTP id 2-20020ac85902000000b003ba19e53e45mr4439204qty.13.1676389973474; Tue, 14 Feb 2023 07:52:53 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id s15-20020ac85ecf000000b003b62e8b77e7sm11275962qtx.68.2023.02.14.07.52.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Feb 2023 07:52:53 -0800 (PST) From: Andrew Burgess To: Tom Tromey via Gdb-patches , gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCH] Do not cast away const in agent_run_command In-Reply-To: <20230214140531.3374817-1-tromey@adacore.com> References: <20230214140531.3374817-1-tromey@adacore.com> Date: Tue, 14 Feb 2023 15:52:51 +0000 Message-ID: <878rh0megc.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tom Tromey via Gdb-patches writes: > While investigating something else, I noticed some weird code in > agent_run_command (use of memcpy rather than strcpy). Then I noticed > that 'cmd' is used as both an in and out parameter, despite being > const. > > Casting away const like this is bad. This patch removes the const and > fixes the memcpy. I also added a static assert to assure myself that > the code in gdbserver is correct -- gdbserver is passing its own > buffer directly to agent_run_command. LGTM. Reviewed-By: Andrew Burgess Thanks, Andrew > --- > gdb/linux-nat.c | 7 ++----- > gdbserver/server.cc | 5 +++++ > gdbserver/tracepoint.cc | 2 +- > gdbsupport/agent.cc | 14 ++++++++------ > gdbsupport/agent.h | 2 +- > 5 files changed, 17 insertions(+), 13 deletions(-) > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index 2b206a4ec1e..d6e69e61ef5 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -4114,9 +4114,7 @@ linux_nat_target::static_tracepoint_markers_by_strid (const char *strid) > /* Pause all */ > target_stop (ptid); > > - memcpy (s, "qTfSTM", sizeof ("qTfSTM")); > - s[sizeof ("qTfSTM")] = 0; > - > + strcpy (s, "qTfSTM"); > agent_run_command (pid, s, strlen (s) + 1); > > /* Unpause all. */ > @@ -4133,8 +4131,7 @@ linux_nat_target::static_tracepoint_markers_by_strid (const char *strid) > } > while (*p++ == ','); /* comma-separated list */ > > - memcpy (s, "qTsSTM", sizeof ("qTsSTM")); > - s[sizeof ("qTsSTM")] = 0; > + strcpy (s, "qTsSTM"); > agent_run_command (pid, s, strlen (s) + 1); > p = s; > } > diff --git a/gdbserver/server.cc b/gdbserver/server.cc > index 21fb51a45d1..46dfe70838b 100644 > --- a/gdbserver/server.cc > +++ b/gdbserver/server.cc > @@ -51,6 +51,11 @@ > #include "gdbsupport/scoped_restore.h" > #include "gdbsupport/search.h" > > +/* PBUFSIZ must also be at least as big as IPA_CMD_BUF_SIZE, because > + the client state data is passed directly to some agent > + functions. */ > +gdb_static_assert (PBUFSIZ >= IPA_CMD_BUF_SIZE); > + > #define require_running_or_return(BUF) \ > if (!target_running ()) \ > { \ > diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc > index 37a9a8c5b7c..b59077a3896 100644 > --- a/gdbserver/tracepoint.cc > +++ b/gdbserver/tracepoint.cc > @@ -6820,7 +6820,7 @@ run_inferior_command (char *cmd, int len) > target_pause_all (false); > uninsert_all_breakpoints (); > > - err = agent_run_command (pid, (const char *) cmd, len); > + err = agent_run_command (pid, cmd, len); > > reinsert_all_breakpoints (); > target_unpause_all (false); > diff --git a/gdbsupport/agent.cc b/gdbsupport/agent.cc > index 531807be3d2..81c925dd99d 100644 > --- a/gdbsupport/agent.cc > +++ b/gdbsupport/agent.cc > @@ -179,14 +179,16 @@ gdb_connect_sync_socket (int pid) > #endif > } > > -/* Execute an agent command in the inferior. PID is the value of pid of the > - inferior. CMD is the buffer for command. GDB or GDBserver will store the > - command into it and fetch the return result from CMD. The interaction > - between GDB/GDBserver and the agent is synchronized by a synchronization > - socket. Return zero if success, otherwise return non-zero. */ > +/* Execute an agent command in the inferior. PID is the value of pid > + of the inferior. CMD is the buffer for command. It is assumed to > + be at least IPA_CMD_BUF_SIZE bytes long. GDB or GDBserver will > + store the command into it and fetch the return result from CMD. > + The interaction between GDB/GDBserver and the agent is synchronized > + by a synchronization socket. Return zero if success, otherwise > + return non-zero. */ > > int > -agent_run_command (int pid, const char *cmd, int len) > +agent_run_command (int pid, char *cmd, int len) > { > int fd; > int tid = agent_get_helper_thread_id (); > diff --git a/gdbsupport/agent.h b/gdbsupport/agent.h > index dceb33f6bd5..7a258e267a5 100644 > --- a/gdbsupport/agent.h > +++ b/gdbsupport/agent.h > @@ -22,7 +22,7 @@ > > #include "gdbsupport/preprocessor.h" > > -int agent_run_command (int pid, const char *cmd, int len); > +int agent_run_command (int pid, char *cmd, int len); > > int agent_look_up_symbols (void *); > > -- > 2.39.1