From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754679AbbKXRiY (ORCPT ); Tue, 24 Nov 2015 12:38:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41859 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754071AbbKXRiW (ORCPT ); Tue, 24 Nov 2015 12:38:22 -0500 Subject: Re: [Qemu-devel] [PATCH v5 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device To: "Gabriel L. Somlo" , kbuild test robot References: <1448294264-17388-2-git-send-email-somlo@cmu.edu> <201511240404.AFpczj7x%fengguang.wu@intel.com> <20151124165553.GA22627@HEDWIG.INI.CMU.EDU> Cc: mark.rutland@arm.com, peter.maydell@linaro.org, mst@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, eric@anholt.net, kraxel@redhat.com, linux-api@vger.kernel.org, pawel.moll@arm.com, zajec5@gmail.com, galak@codeaurora.org, rmk+kernel@arm.linux.org.uk, lersek@redhat.com, hanjun.guo@linaro.org, devicetree@vger.kernel.org, arnd@arndb.de, ijc+devicetree@hellion.org.uk, jordan.l.justen@intel.com, agross@codeaurora.org, leif.lindholm@linaro.org, robh+dt@kernel.org, ard.biesheuvel@linaro.org, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, luto@amacapital.net, kbuild-all@01.org, sudeep.holla@arm.com, pbonzini@redhat.com, revol@free.fr From: Eric Blake Openpgp: url=http://people.redhat.com/eblake/eblake.gpg X-Enigmail-Draft-Status: N1110 Organization: Red Hat, Inc. Message-ID: <5654A08A.6030002@redhat.com> Date: Tue, 24 Nov 2015 10:38:18 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151124165553.GA22627@HEDWIG.INI.CMU.EDU> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/24/2015 09:55 AM, Gabriel L. Somlo wrote: > On Tue, Nov 24, 2015 at 04:14:50AM +0800, kbuild test robot wrote: >> >> drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_set': >>>> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects= argument of type 'long long int *', but argument 3 has type 'phys_addr_t= *' [-Wformat=3D] >> &ctrl_off, &data_off, &consumed); >> ^ >=20 > Oh, I think I know why this happened: >=20 >=20 > So, I could use u64 instead of phys_addr_t and resource_size_t, and > keep "%lli" (or "%Li"), but then I'd have to check if the parsed value %Li is not POSIX. Don't use it (stick with %lli). > would overflow a 32-bit address value on arches where phys_addr_t is > u32, which would make things a bit more messy and awkward. >=20 > I'm planning on #ifdef-ing the format string instead: >=20 > #ifdef CONFIG_PHYS_ADDR_T_64BIT > #define PH_ADDR_SCAN_FMT "@%Li%n:%Li:%Li%n" > #else > #define PH_ADDR_SCAN_FMT "@%li%n:%li:%li%n" > #endif A more typical approach is akin to ; have PH_ADDR_FMT defined to either "lli" or "li", then write sscanf(str, "@%"PH_ADDR_FMT "%n:..., ...), using PH_ADDR_FMT multiple times. > ... > processed =3D sscanf(str, PH_ADDR_SCAN_FMT, > &base, &consumed, > &ctrl_off, &data_off, &consumed); Umm, why are you passing &consumed to more than one sscanf() %? That's (probably) undefined behavior. [In general, sscanf() is a horrid interface to use for parsing integers - it has undefined behavior if the input text would trigger integer overflow, making it safe to use ONLY on text that you control and can guarantee won't overflow. By the time you've figured out if untrusted text meets the requirement for safe parsing via sscanf(), you've practically already parsed it via safer strtol() and friends.] --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWVKCKAAoJEKeha0olJ0Nq4kgH/RnVbh6yH6EpDAFjVIauu0D7 0Z9gc2FHZuQhNBl1bCWXx8wtXKbDtsR2RRCIIEF/I0KWVLt2G6u9tyV5ywjnej8x jAGQLpjR748H/H19vAHDp8U/v+6hpIfEscWjIFHwlMWzxOQ6cOLDTD24S/8CRhDf /luUrFEkpzY4aL3o9aBlwcNjFv5lmdxU+TmehDcsAKXbJtC/ZvXku8j6umavfL+Z Q0APbHZsG+gB9aA9w5nDMaubySkIe97rfrxqOROdFp1kK+IHpglIasWovYAoe4fJ rBGlSDvP765pfq7PoK0od9W+FQ3ZsQma2PTiTs41kXrj+tOg1zmOhAEbbmpwEQQ= =NNe0 -----END PGP SIGNATURE----- --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Blake Subject: Re: [Qemu-devel] [PATCH v5 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device Date: Tue, 24 Nov 2015 10:38:18 -0700 Message-ID: <5654A08A.6030002@redhat.com> References: <1448294264-17388-2-git-send-email-somlo@cmu.edu> <201511240404.AFpczj7x%fengguang.wu@intel.com> <20151124165553.GA22627@HEDWIG.INI.CMU.EDU> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6" Return-path: In-Reply-To: <20151124165553.GA22627-h65ZQ0r4j6KKUezXOiBB2eW1CriLhL8O@public.gmane.org> Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Gabriel L. Somlo" , kbuild test robot Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org, peter.maydell-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, stefanha-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, qemu-devel-qX2TKyscuCcdnm+yROfE0A@public.gmane.org, eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org, kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, pawel.moll-5wv7dgnIgG8@public.gmane.org, zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org, lersek-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, arnd-r2nGTMty4D4@public.gmane.org, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org, jordan.l.justen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, agross-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org, kbuild-all-JC7UmRfGjtg@public.gmane.org, sudeep.holla-5wv7dgnIgG8@public.gmane.org, pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, revol-GANU6spQydw@public.gmane.org List-Id: devicetree@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/24/2015 09:55 AM, Gabriel L. Somlo wrote: > On Tue, Nov 24, 2015 at 04:14:50AM +0800, kbuild test robot wrote: >> >> drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_set': >>>> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects= argument of type 'long long int *', but argument 3 has type 'phys_addr_t= *' [-Wformat=3D] >> &ctrl_off, &data_off, &consumed); >> ^ >=20 > Oh, I think I know why this happened: >=20 >=20 > So, I could use u64 instead of phys_addr_t and resource_size_t, and > keep "%lli" (or "%Li"), but then I'd have to check if the parsed value %Li is not POSIX. Don't use it (stick with %lli). > would overflow a 32-bit address value on arches where phys_addr_t is > u32, which would make things a bit more messy and awkward. >=20 > I'm planning on #ifdef-ing the format string instead: >=20 > #ifdef CONFIG_PHYS_ADDR_T_64BIT > #define PH_ADDR_SCAN_FMT "@%Li%n:%Li:%Li%n" > #else > #define PH_ADDR_SCAN_FMT "@%li%n:%li:%li%n" > #endif A more typical approach is akin to ; have PH_ADDR_FMT defined to either "lli" or "li", then write sscanf(str, "@%"PH_ADDR_FMT "%n:..., ...), using PH_ADDR_FMT multiple times. > ... > processed =3D sscanf(str, PH_ADDR_SCAN_FMT, > &base, &consumed, > &ctrl_off, &data_off, &consumed); Umm, why are you passing &consumed to more than one sscanf() %? That's (probably) undefined behavior. [In general, sscanf() is a horrid interface to use for parsing integers - it has undefined behavior if the input text would trigger integer overflow, making it safe to use ONLY on text that you control and can guarantee won't overflow. By the time you've figured out if untrusted text meets the requirement for safe parsing via sscanf(), you've practically already parsed it via safer strtol() and friends.] --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWVKCKAAoJEKeha0olJ0Nq4kgH/RnVbh6yH6EpDAFjVIauu0D7 0Z9gc2FHZuQhNBl1bCWXx8wtXKbDtsR2RRCIIEF/I0KWVLt2G6u9tyV5ywjnej8x jAGQLpjR748H/H19vAHDp8U/v+6hpIfEscWjIFHwlMWzxOQ6cOLDTD24S/8CRhDf /luUrFEkpzY4aL3o9aBlwcNjFv5lmdxU+TmehDcsAKXbJtC/ZvXku8j6umavfL+Z Q0APbHZsG+gB9aA9w5nDMaubySkIe97rfrxqOROdFp1kK+IHpglIasWovYAoe4fJ rBGlSDvP765pfq7PoK0od9W+FQ3ZsQma2PTiTs41kXrj+tOg1zmOhAEbbmpwEQQ= =NNe0 -----END PGP SIGNATURE----- --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1HXv-0002eR-3f for qemu-devel@nongnu.org; Tue, 24 Nov 2015 12:38:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1HXr-0006Zi-20 for qemu-devel@nongnu.org; Tue, 24 Nov 2015 12:38:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49091) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1HXq-0006ZY-Qr for qemu-devel@nongnu.org; Tue, 24 Nov 2015 12:38:22 -0500 References: <1448294264-17388-2-git-send-email-somlo@cmu.edu> <201511240404.AFpczj7x%fengguang.wu@intel.com> <20151124165553.GA22627@HEDWIG.INI.CMU.EDU> From: Eric Blake Message-ID: <5654A08A.6030002@redhat.com> Date: Tue, 24 Nov 2015 10:38:18 -0700 MIME-Version: 1.0 In-Reply-To: <20151124165553.GA22627@HEDWIG.INI.CMU.EDU> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6" Subject: Re: [Qemu-devel] [PATCH v5 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gabriel L. Somlo" , kbuild test robot Cc: mark.rutland@arm.com, peter.maydell@linaro.org, mst@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, linux-kernel@vger.kernel.org, eric@anholt.net, kraxel@redhat.com, pawel.moll@arm.com, zajec5@gmail.com, rmk+kernel@arm.linux.org.uk, lersek@redhat.com, kbuild-all@01.org, devicetree@vger.kernel.org, arnd@arndb.de, ijc+devicetree@hellion.org.uk, jordan.l.justen@intel.com, galak@codeaurora.org, leif.lindholm@linaro.org, robh+dt@kernel.org, sudeep.holla@arm.com, ard.biesheuvel@linaro.org, linux-api@vger.kernel.org, gregkh@linuxfoundation.org, luto@amacapital.net, hanjun.guo@linaro.org, agross@codeaurora.org, pbonzini@redhat.com, revol@free.fr This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/24/2015 09:55 AM, Gabriel L. Somlo wrote: > On Tue, Nov 24, 2015 at 04:14:50AM +0800, kbuild test robot wrote: >> >> drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_set': >>>> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects= argument of type 'long long int *', but argument 3 has type 'phys_addr_t= *' [-Wformat=3D] >> &ctrl_off, &data_off, &consumed); >> ^ >=20 > Oh, I think I know why this happened: >=20 >=20 > So, I could use u64 instead of phys_addr_t and resource_size_t, and > keep "%lli" (or "%Li"), but then I'd have to check if the parsed value %Li is not POSIX. Don't use it (stick with %lli). > would overflow a 32-bit address value on arches where phys_addr_t is > u32, which would make things a bit more messy and awkward. >=20 > I'm planning on #ifdef-ing the format string instead: >=20 > #ifdef CONFIG_PHYS_ADDR_T_64BIT > #define PH_ADDR_SCAN_FMT "@%Li%n:%Li:%Li%n" > #else > #define PH_ADDR_SCAN_FMT "@%li%n:%li:%li%n" > #endif A more typical approach is akin to ; have PH_ADDR_FMT defined to either "lli" or "li", then write sscanf(str, "@%"PH_ADDR_FMT "%n:..., ...), using PH_ADDR_FMT multiple times. > ... > processed =3D sscanf(str, PH_ADDR_SCAN_FMT, > &base, &consumed, > &ctrl_off, &data_off, &consumed); Umm, why are you passing &consumed to more than one sscanf() %? That's (probably) undefined behavior. [In general, sscanf() is a horrid interface to use for parsing integers - it has undefined behavior if the input text would trigger integer overflow, making it safe to use ONLY on text that you control and can guarantee won't overflow. By the time you've figured out if untrusted text meets the requirement for safe parsing via sscanf(), you've practically already parsed it via safer strtol() and friends.] --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWVKCKAAoJEKeha0olJ0Nq4kgH/RnVbh6yH6EpDAFjVIauu0D7 0Z9gc2FHZuQhNBl1bCWXx8wtXKbDtsR2RRCIIEF/I0KWVLt2G6u9tyV5ywjnej8x jAGQLpjR748H/H19vAHDp8U/v+6hpIfEscWjIFHwlMWzxOQ6cOLDTD24S/8CRhDf /luUrFEkpzY4aL3o9aBlwcNjFv5lmdxU+TmehDcsAKXbJtC/ZvXku8j6umavfL+Z Q0APbHZsG+gB9aA9w5nDMaubySkIe97rfrxqOROdFp1kK+IHpglIasWovYAoe4fJ rBGlSDvP765pfq7PoK0od9W+FQ3ZsQma2PTiTs41kXrj+tOg1zmOhAEbbmpwEQQ= =NNe0 -----END PGP SIGNATURE----- --sImjOk8MwI0j6lP7WkPsFtqx8wGB2nOe6--