From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mta-04.yadro.com (mta-04.yadro.com [89.207.88.248]) by sourceware.org (Postfix) with ESMTPS id 73F263858D38 for ; Wed, 10 Jan 2024 14:58:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 73F263858D38 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=syntacore.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=syntacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 73F263858D38 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=89.207.88.248 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704898687; cv=none; b=FMY5EIwRuNeIB2GdFQEBusFRoDbKmmu8hsD5Zhrv47Hwc/zv7QlE4RZiJmuwFLAGP7+wIJHAE75oDltQxhcEUcBqJ/wR07Kwn+NOA7/MymBBSEHEXXB3x/xIQbOHVMnvyDMYLoCs3PWWgTBQ5LXoFYC4QfgI4RMqWioTq1UhkCs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704898687; c=relaxed/simple; bh=6O7y2J6DVr0Z00GX3wtI74o2J7u0JPkA4Amx3K9wbXg=; h=DKIM-Signature:DKIM-Signature:From:To:Subject:Date:Message-ID: MIME-Version; b=ClM6brJ3ypci3YgwDTuvFT6UD9IEYjFRJ2UAumwvW8VqhJqQEWrbY6R9XXh16qaRftY73hs4dtT72CSDnW2cTEj8TJ288Sx6sRX15TsmdW6KY4sHIj/V7rlNA3LU0mRTxPIMxu+Cy7oxbAJEke9bXshptgEGfobOoq78YIn4kKo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 mta-04.yadro.com 0A1A8C0002 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=syntacore.com; s=mta-04; t=1704898682; bh=5UCCrPOfgLVHkecwbi+sQ4oCf7qyGw021lvo675/abM=; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type:From; b=AXBQ9WZq5GdEnbbBwGHGAAkOB6opnf3OgeGIRvJzIAozpQwg29CjpUrVE58FYZJPA Wol7c133PEGCHNUZPyVOfcWtSwIojNw86jprLSQLFFpIbDxQuGtQwhOn+wbbjC8am6 lD2XoZqeHnK/sQXRrNDk1udj0saT1rzRArb2M3FL3GK9eQFRTMWAiSthojiJLMwvt4 ugN3p04Yqx3FBkEmpcpxuPoFReu3XYMA/itUaMSWAS5srGLZPwSZr9M/E5mP7KrSDn 22PqlGdswh7ICNTw/jvHg55JR26fi0TIL1r38O87JIqZUsBB9+bRxHES1hQxlE5qM3 A7gTRT4oDeh+w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=syntacore.com; s=mta-03; t=1704898682; bh=5UCCrPOfgLVHkecwbi+sQ4oCf7qyGw021lvo675/abM=; h=From:To:Subject:Date:Message-ID:MIME-Version:Content-Type:From; b=UOBaXVfJisvOy9LwGny8sp0K0rwb0iYYIrjN1oT0n4JhQxUIc9yg94K0fy6WizDhy xrKn7MybwK6YjnDygr3ZkCqKPzbD8BhZhsao1ruViaN2blqUDx6qQsdRvgfj1KSOFM KFYL3FY415t0Yk/lXtoRNZ0LsvkgNp9cnO6A5EkqDxIFZULz04hCiwhgcrCUGg4q9w iE/zHwZucHfX6QgW/3P8G8YigHQjXxMmI9bScYFaEYr1vjCGY05hRIaWqy8+Yz7Rre nbw25zB8heWbSNFhlKo6o5MDTqYNbOc15lPEF5OcU1wSmZr3xhXs86BkXfNsgQvbFx z1/rjKiA9LQ+A== From: Kirill Radkin To: , CC: , , , Kirill Radkin Subject: [PATCH] gdbserver: Fix overflow detection in gdbserver Date: Wed, 10 Jan 2024 17:57:46 +0300 Message-ID: <20240110145746.57459-1-kirill.radkin@syntacore.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <87edfe2qiy.fsf@tromey.com> References: <87edfe2qiy.fsf@tromey.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-ClientProxiedBy: T-EXCH-06.corp.yadro.com (172.17.10.110) To S-Exch-01.corp.yadro.com (10.78.5.241) X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,SPF_HELO_NONE,TXREP,T_SCC_BODY_TEXT_LINE,T_SPF_PERMERROR 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: > First, the boring stuff: I think that the patch is more than an obvious > fix, so we'll need to make sure you are covered with an FSF copyright > assignment [1]. Do you know if you (or your employer, since it looks > like you're doing this as part of your employment) are covered with one > already? Yes, we recieved assignment on Dec 26 2022 (RT:1894351). Accepted by John Hsieh. > Then, I am unable to apply your patch with git-am. It looks like you > copy pasted the patch in your email client, which usually gives bad > results. Are you able to use git-send-email [2]? This is the most > trustworthy way. Done. > Does this create a sparse file on systems that support it? I could see > it as a problem for some test environments if we create and leave a file > occupying ~4GB file in the build directory. Well, two actually, there > will be the .o and the final executable. Note that I don't have a > better idea on how to test this... The only way to test my patch is transferring large (>2GB) binaries. I decided to create large binary this way. > Do you expect this .S file to work for all architectures? It's simple, > but I guess that the "ret" instruction doesn't exist on all > architectures. Can you think of a way to produce a big binary that is > not architecture-dependent at all? Maybe produce an executable, and > then insert a large dummy section in the beginning? "ret" removed. I have simplified binary, so now it should work for all architectures. Currently gdbserver uses require_int() function to parse the requested offset (in vFile::pread packet and the like). This function allows integers up to 0x7fffffff (to fit in 32-bit int), however the offset (for pread system call) has an off_t type which can be larger than 32-bit. This patch allows require_int() function to parse offset up to the maximum value implied by the off_t type. --- gdb/testsuite/gdb.server/pread-offset-size.S | 23 +++++++++ .../gdb.server/pread-offset-size.exp | 50 +++++++++++++++++++ gdbserver/hostio.cc | 19 +++++-- 3 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.S create mode 100644 gdb/testsuite/gdb.server/pread-offset-size.exp diff --git a/gdb/testsuite/gdb.server/pread-offset-size.S b/gdb/testsuite/gdb.server/pread-offset-size.S new file mode 100644 index 00000000000..b07058e5550 --- /dev/null +++ b/gdb/testsuite/gdb.server/pread-offset-size.S @@ -0,0 +1,23 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2023 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + + .text + .globl _start +_start: + .skip 3742415472 + .globl f +f: diff --git a/gdb/testsuite/gdb.server/pread-offset-size.exp b/gdb/testsuite/gdb.server/pread-offset-size.exp new file mode 100644 index 00000000000..221491bfa04 --- /dev/null +++ b/gdb/testsuite/gdb.server/pread-offset-size.exp @@ -0,0 +1,50 @@ +# Copyright (C) 2023 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# Test sending of large binary files (>2 GB) from gdbserver +# (it was unavailable earlier) + +load_lib gdbserver-support.exp + +if {![allow_gdbserver_tests]} { + return +} + +standard_testfile .S + +if { [prepare_for_testing ${testfile}.exp $testfile \ + $srcfile {debug additional_flags=-nostdlib} ] } { + return -1 +} + +gdb_exit +gdb_start + +gdb_test_no_output "set remote exec-file $binfile" \ +"set remote exec-file" + +# Make sure we're disconnected, in case we're testing with an +# extended-remote board, therefore already connected. +gdb_test "disconnect" ".*" + +set res [gdbserver_spawn ""] +set gdbserver_protocol [lindex $res 0] +set gdbserver_gdbport [lindex $res 1] + +gdb_test "target $gdbserver_protocol $gdbserver_gdbport" \ +"Remote debugging using .*" \ +"target $gdbserver_protocol $gdbserver_gdbport" + +gdb_test "break f" "Breakpoint 1.*" diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc index ea70c26da0f..d27ec58740d 100644 --- a/gdbserver/hostio.cc +++ b/gdbserver/hostio.cc @@ -90,12 +90,20 @@ require_filename (char **pp, char *filename) return 0; } +template static int -require_int (char **pp, int *value) +require_int (char **pp, T *value) { + constexpr bool is_signed = std::is_signed::value; + if (!is_signed) + return -1; + char *p; int count, firstdigit; + /* Max count of hexadecimal digits in off_t (1 hex digit is 4 bits) */ + int max_count = sizeof(T) * CHAR_BIT / 4; + p = *pp; *value = 0; count = 0; @@ -112,7 +120,8 @@ require_int (char **pp, int *value) firstdigit = nib; /* Don't allow overflow. */ - if (count >= 8 || (count == 7 && firstdigit >= 0x8)) + if (count >= max_count || (count == (max_count - 1) + && firstdigit >= 0x8)) return -1; *value = *value * 16 + nib; @@ -344,7 +353,8 @@ handle_open (char *own_buf) static void handle_pread (char *own_buf, int *new_packet_len) { - int fd, ret, len, offset, bytes_sent; + int fd, ret, len, bytes_sent; + off_t offset; char *p, *data; static int max_reply_size = -1; @@ -411,7 +421,8 @@ handle_pread (char *own_buf, int *new_packet_len) static void handle_pwrite (char *own_buf, int packet_len) { - int fd, ret, len, offset; + int fd, ret, len; + off_t offset; char *p, *data; p = own_buf + strlen ("vFile:pwrite:"); -- 2.34.1