lkml.org 
[lkml]   [2015]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Qemu-devel] [PATCH v5 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
From
Date
On 11/24/15 18:38, Eric Blake wrote:
> 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=]
>>> &ctrl_off, &data_off, &consumed);
>>> ^
>>
>> Oh, I think I know why this happened:
>>
>
>>
>> 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.
>>
>> I'm planning on #ifdef-ing the format string instead:
>>
>> #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 <inttypes.h>; 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 = 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.]
>

Yes, but this is the kernel, which may or may not follow POSIX
semantics. (And may or may not curse at POSIX in the process, either
way! :))

Laszlo


\
 
 \ /
  Last update: 2015-11-24 19:21    [W:0.043 / U:0.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site