- TF-A - lists.trustedfirmware.org

Re: [TF-A] [RFC] New feature in TF-A to load encrypted FIP payloads

by Sandrine Bailleux

Hi Sumit, I've started providing some early review comments on your patches in Gerrit but these are mainly about implementation details. I would like to continue the higher-level design discussions here in parallel on the mailing list, if that's fine with you. On 12/6/19 1:29 PM, Sumit Garg via TF-A wrote: > Hi Sandrine, > > On Thu, 5 Dec 2019 at 20:31, Sandrine Bailleux > <Sandrine.Bailleux(a)arm.com> wrote: >>>> Also, I am still trying to get my head around how this would integrate >>>> with a cryptographic engine where the key does not leave the chip. I can >>>> imagine that we could get the address of the encrypted firmware image >>>> from the FIP, pass that to a cryptographic engine, request it to decrypt >>>> it and store the result somewhere in Trusted RAM. In this case, we >>>> wouldn't call plat_get_fip_encryption_key(). Do you have any idea how we >>>> would pull this off? Like how the different modules (IO layer, crypto >>>> module, image parser module, ...) would integrate together? >>> >>> In this case, I would expect platform to provide key identifier rather >>> than actual key as part of plat_get_fip_encryption_key() which is then >>> passed onto auth_decrypt() that is implementation specific to >>> cryptographic engine in similar terms as currently done for mbedTLS >>> backend. >> >> Ah I see, so plat_get_fip_encryption_key() could return either the key >> itself or a key identifier. Just like plat_get_rotpk_info() can return >> either the key or a hash of it today. However, in the case of >> plat_get_rotpk_info(), it also returns some flags indicating which one >> it is. Don't we need something similar for >> plat_get_fip_encryption_key()? How will the crypto module be able to >> tell the difference between a key and a key ID otherwise? Or would you >> expect a given crypto module backend to always use either keys or key >> IDs, but not both? >> > > It perfectly makes sense for crypto module backend to support both key > ID and actual key. But I was thinking from use-case perspective that > why would one like to expose key to the firmware if the crypto module > could better protect it. And I could come up with one use-case where > the SSK (burnt in SoC fuses) is provided as an actual key and BSSK > (accessible only to crypto module) is provided via a key ID. Yes, that makes sense to me. > So yes I agree with you to provide additional flags field along with > key buffer. Also, I think it makes sense to rename > "plat_get_fip_encryption_key()" to "plat_get_fip_enc_key_info()". Agree. >>>> I have some concerns around the generation of the initialization vectors >>>> in the encrypt_fw tool. Right now, IVs are simply a random sequence of >>>> bytes (obtained through a call to OpenSSL's RAND_bytes() API). Now, I >>>> would imagine that RAND_bytes() is typically based on a good random >>>> number generator and thus will generate different sequences every time >>>> it is called. At least, as long as it is called from the same machine >>>> every time. But what if we encrypt a new FIP bundle from a different >>>> machine, say in the context of a firmware update? Is it not possible >>>> that it might choose the same IV out of bad luck? >>>> >>>> Perhaps that's an issue left to provisioning/manufacturing time and is >>>> out of the scope here. But it worries me because AFAIU, the security of >>>> AES-GCM is critically undermined if the same nonce is used multiple >>>> times with the same key (see section 5.1.1. "Nonce reuse" in RFC 5116). >>>> If the encryption key is the SSK (rather than the BSSK) then I guess the >>>> probability is even higher, as it is shared amongst a class of devices. >>>> >>> >>> Agree that "nonce" should be unique and using a random number >>> generator available on build machine was an effort towards that. But >>> thinking about the case that you have mentioned, I think we could have >>> an optional user provided "nonce" as an input to "encrypt_fw" tool, so >>> that the user is aware to randomly generate and provide a unique >>> nonce. >> >> Yes, I like your suggestion of specifying the nonce to the tool. But I >> think it should be the default behaviour then. You mention an *optional* >> user-provided nonce, I would like to suggest we make this mandatory >> instead. We could provide an option to request the tool to generate the >> nonce, intended for development purposes. >> > > Making nonce as default and mandatory command line option seems to be > overkill for a user. It may also lead to unsafe practices of > hardcoding a nonce in Makefile, shell scripts etc. So I think we > should keep this as optional only for a narrow use-case where people > might not trust uniqueness property of RNG from multiple build > machines. I don't know... Hard-coding a nonce in the makefile is unsafe for sure but I would think that a nonce generated by the tool is also unsafe without proper management of these nonces across keys and devices, but in a much more subtle manner. I still think the nonce generation would be better left outside of the scope of this tool. > BTW, I have already added nonce command line option as part of v3 > patch-set [1]. Please have a look. > > [1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2496/2..3 > >>>> Impact on memory footprint and performance >>>> ------------------------------------------ >>>> >>>> Do you know what the performance impact is when this feature is enabled >>>> in TF-A, to decrypt images at boot time? Obviously it depends on the >>>> platform and whether there is a dedicated cryptographic engine, and I >>>> suppose you cannot really get any relevant measurements out of QEMU but >>>> I would be interested if you've got any rough numbers. >>> >>> Following are measurements based on qemu for mbedTLS software library >>> based authenticated decryption: >>> >>> BL31 plain: >>> NOTICE: Load image time: 137us, size: 28KB >>> BL31 encrypted: >>> NOTICE: Load image time: 3979us, size: 28KB >>> >>> BL32 plain: >>> NOTICE: Load image time: 1791us, size: 360KB >>> BL32 encrypted: >>> NOTICE: Load image time: 36339us, size: 360KB >> >> Thanks. So it's a 29% increase for BL31 and 20% for BL32. I would have >> naively expected similar percentages, any idea why we get such a >> difference between BL31 and BL32? I am just curious. Maybe it's down to >> how the crypto algorithm/mode of operation works under the hood? > > Yes this additional increase for BL31 seems to be due to one time > initialization operations carried on first invocation of mbedTLS gcm > APIs, especially time taken to generate AES tables (see call to > aes_gen_tables() in mbedtls/library/aes.c +576). Ah I see, thanks for the explanation. Regarding the memory impact, given that it's adding quite a bit of code and data (around 9KB for BL1 and 12KB for BL2 from what you were saying in a previous email), I think we need to make this feature optional and wrap it under a build flag. Not everyone will need to encrypt firmware images, having only authentication is a valid use case as well. If I am not mistaken, your patches right now unconditionally pull the mbedTLS primitives into BL1 and BL2 (as long as TBBR is enabled of course). As a matter of fact, I am not able to build BL1 on FVP platform with your patches, presumably due to the unconditional size increase. The following build command: > make TRUSTED_BOARD_BOOT=1 ARM_ROTPK_LOCATION=devel_rsa MBEDTLS_DIR=mbedtls bl1 ... fails at link time: aarch64-linux-gnu-ld.bfd: build/fvp/release/bl1/bl1.elf section `xlat_table' will not fit in region `RAM' aarch64-linux-gnu-ld.bfd: BL1's RW section has exceeded its limit. aarch64-linux-gnu-ld.bfd: region `RAM' overflowed by 4096 bytes Makefile:889: recipe for target 'build/fvp/release/bl1/bl1.elf' failed Regards, Sandrine

6 years

2
2
0 0

Re: [TF-A] [Question] Why are the log related macros defined in multiple of 10's

by Dan Handley

Hi > -----Original Message----- > From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> On Behalf Of Soby Mathew > via TF-A > Sent: 06 January 2020 12:34 > To: Olivier Deprez <Olivier.Deprez(a)arm.com>; tf-a(a)lists.trustedfirmware.org; > Sheetal Tigadoli <sheetal.tigadoli(a)broadcom.com> > Cc: nd <nd(a)arm.com> > Subject: Re: [TF-A] [Question] Why are the log related macros defined in > multiple of 10's > > > Hi All, > > > > I wanted to understand the reason behind defining LOG_LEVEL_* macros > > as multiple of 10's, also same goes with LOG_MARKER_* macros. > > If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and > > division operation when printing the msgs to console. > > > > Please let me your thoughts on this. > > > > Link to this question on Phabricator - > > https://developer.trustedfirmware.org/T624 > > > > Thanks > > Sheetal > As Sandrine said in Phabricator, the reason was simply to leave space between the levels to allow adding intermediate levels without breaking compatibility. Compatibility was a possible concern if external tools used the log level and you only rebuilt some of the TF-A components (components could end up using different values). However, I suspect the actual level value is never emitted in practice (just the text prefix) so this may not be a real concern. > Hi Sheetal > I didn't quite get reason for the optimization from multiples of 10 to > consecutive numbers. The mod operation you refer to are assertions which will > be compiled out when DEBUG=0 or when ENABLE_ASSSERTIONS=0. Other than that > there is a single divide by 10 operation within the default implementation of > plat_log_get_prefix(). > > > If the logging utility is considered performance critical, removing the > single divide by 10 will not result in any gain. Usually the bottleneck for > logs are UART baudrates and the CPU spends most of time spinning in a wait > loop for UART to be free. Hence I don't see any need for change for log > levels macro if the intent is performance optimization. > +1. Unless there's an actual performance gain, this sounds like an unnecessary change. Dan. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

6 years

1
0
0 0

Re: [TF-A] [Question] Why are the log related macros defined in multiple of 10's

by Soby Mathew

On 06/01/2020 12:34, Soby Mathew via TF-A wrote: >> Hi All, >> >> I wanted to understand the reason behind defining LOG_LEVEL_* macros as >> multiple of 10's, also same goes with LOG_MARKER_* macros. >> If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and >> division operation when printing the msgs to console. >> >> Please let me your thoughts on this. >> >> Link to this question on Phabricator - >> https://developer.trustedfirmware.org/T624 >> >> Thanks >> Sheetal > > Hi Sheetal > I didn't quite get reason for the optimization from multiples of 10 to > consecutive numbers. The mod operation you refer to are assertions which > will be compiled out when DEBUG=0 or when ENABLE_ASSSERTIONS=0. Other > than that there is a single divide by 10 operation within the default > implementation of plat_log_get_prefix(). > > > If the logging utility is considered performance critical, removing the > single divide by 10 will not result in any gain. Usually the bottleneck > for logs are UART baudrates and the CPU spends most of time spinning in > a wait loop for UART to be free. Hence I don't see any need for change > for log levels macro if the intent is performance optimization. > > Best Regards > Soby Mathew > Hi Sheetal There is some discussion related to this here : https://github.com/ARM-software/tf-issues/issues/462 and see associated issues and PRs for more details. Best Regards Soby Mathew >> -- >> TF-A mailing list >> TF-A(a)lists.trustedfirmware.org >> https://lists.trustedfirmware.org/mailman/listinfo/tf-a >> >

6 years, 1 month

1
0
0 0

Re: [TF-A] [Question] Why are the log related macros defined in multiple of 10's

by Soby Mathew

> Hi All, > > I wanted to understand the reason behind defining LOG_LEVEL_* macros as > multiple of 10's, also same goes with LOG_MARKER_* macros. > If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and > division operation when printing the msgs to console. > > Please let me your thoughts on this. > > Link to this question on Phabricator - > https://developer.trustedfirmware.org/T624 > > Thanks > Sheetal Hi Sheetal I didn't quite get reason for the optimization from multiples of 10 to consecutive numbers. The mod operation you refer to are assertions which will be compiled out when DEBUG=0 or when ENABLE_ASSSERTIONS=0. Other than that there is a single divide by 10 operation within the default implementation of plat_log_get_prefix(). If the logging utility is considered performance critical, removing the single divide by 10 will not result in any gain. Usually the bottleneck for logs are UART baudrates and the CPU spends most of time spinning in a wait loop for UART to be free. Hence I don't see any need for change for log levels macro if the intent is performance optimization. Best Regards Soby Mathew > -- > TF-A mailing list > TF-A(a)lists.trustedfirmware.org > https://lists.trustedfirmware.org/mailman/listinfo/tf-a >

6 years, 1 month

1
0
0 0

New Defects reported by Coverity Scan for ARM-software/arm-trusted-firmware

by scan-admin＠coverity.com

Hi, Please find the latest report on new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan. 9 new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan. 13 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 9 of 9 defect(s) ** CID 353233: Control flow issues (NO_EFFECT) /plat/intel/soc/common/socfpga_sip_svc.c: 235 in is_address_in_ddr_range() ________________________________________________________________________________________________________ *** CID 353233: Control flow issues (NO_EFFECT) /plat/intel/soc/common/socfpga_sip_svc.c: 235 in is_address_in_ddr_range() 229 return false; 230 return true; 231 } 232 233 static bool is_address_in_ddr_range(uint64_t addr) 234 { >>> CID 353233: Control flow issues (NO_EFFECT) >>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "addr >= 0ULL". 235 if (addr >= DRAM_BASE && addr <= DRAM_BASE + DRAM_SIZE) 236 return true; 237 238 return false; 239 } 240 ** CID 353232: Memory - corruptions (OVERRUN) /plat/rockchip/common/drivers/parameter/ddr_parameter.c: 124 in ddr_region_usage_parse() ________________________________________________________________________________________________________ *** CID 353232: Memory - corruptions (OVERRUN) /plat/rockchip/common/drivers/parameter/ddr_parameter.c: 124 in ddr_region_usage_parse() 118 * that means s-regions are all parsed yet, so finsh. 119 */ 120 if (p.ns_top[i] == p.boundary) 121 goto out; 122 123 /* s-region's base starts from previous ns-region's top */ >>> CID 353232: Memory - corruptions (OVERRUN) >>> Overrunning array "p.s_base" of 10 8-byte elements at element index 10 (byte offset 87) using index "p.s_nr" (which evaluates to 10). 124 p.s_base[p.s_nr] = p.ns_top[i]; 125 126 /* s-region's top ends with next ns-region's base */ 127 if (i + 1 < p.ns_nr) 128 p.s_top[p.s_nr] = p.ns_base[i + 1]; 129 else ** CID 353231: (DIVIDE_BY_ZERO) /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 374 in mv_ddr4_dq_vref_calibration() /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 380 in mv_ddr4_dq_vref_calibration() /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 378 in mv_ddr4_dq_vref_calibration() /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 376 in mv_ddr4_dq_vref_calibration() ________________________________________________________________________________________________________ *** CID 353231: (DIVIDE_BY_ZERO) /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 374 in mv_ddr4_dq_vref_calibration() 368 VALIDATE_BUS_ACTIVE(tm->bus_act_mask, subphy_num); 369 vref_avg += center_vref[if_id][subphy_num]; 370 dq_vref_vec[subphy_num] = center_vref[if_id][subphy_num]; 371 vref_subphy_num++; 372 } 373 >>> CID 353231: (DIVIDE_BY_ZERO) >>> In expression "vref_avg / vref_subphy_num", division by expression "vref_subphy_num" which may be zero has undefined behavior. 374 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 375 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_START); 376 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 377 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_END); 378 DEBUG_CALIBRATION(DEBUG_LEVEL_INFO, ("final vref average %d\n", vref_avg / vref_subphy_num)); 379 /* run centralization again with optimal vref to update global structures */ /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 380 in mv_ddr4_dq_vref_calibration() 374 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 375 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_START); 376 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 377 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_END); 378 DEBUG_CALIBRATION(DEBUG_LEVEL_INFO, ("final vref average %d\n", vref_avg / vref_subphy_num)); 379 /* run centralization again with optimal vref to update global structures */ >>> CID 353231: (DIVIDE_BY_ZERO) >>> In expression "vref_avg / vref_subphy_num", division by expression "vref_subphy_num" which may be zero has undefined behavior. 380 mv_ddr4_centralization(dev_num, pbs_tap_factor, c_opt_per_bus, pbs_res_per_bus, valid_win_size, 381 TX_DIR, vref_avg / vref_subphy_num, duty_cycle); 382 } 383 384 /* return dram from vref DRAM from vref training mode */ 385 mv_ddr4_vref_training_mode_ctrl(dev_num, 0, ACCESS_TYPE_MULTICAST, 0); /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 378 in mv_ddr4_dq_vref_calibration() 372 } 373 374 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 375 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_START); 376 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 377 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_END); >>> CID 353231: (DIVIDE_BY_ZERO) >>> In expression "vref_avg / vref_subphy_num", division by expression "vref_subphy_num" which may be zero has undefined behavior. 378 DEBUG_CALIBRATION(DEBUG_LEVEL_INFO, ("final vref average %d\n", vref_avg / vref_subphy_num)); 379 /* run centralization again with optimal vref to update global structures */ 380 mv_ddr4_centralization(dev_num, pbs_tap_factor, c_opt_per_bus, pbs_res_per_bus, valid_win_size, 381 TX_DIR, vref_avg / vref_subphy_num, duty_cycle); 382 } 383 /drivers/marvell/mv_ddr/mv_ddr4_training_calibration.c: 376 in mv_ddr4_dq_vref_calibration() 370 dq_vref_vec[subphy_num] = center_vref[if_id][subphy_num]; 371 vref_subphy_num++; 372 } 373 374 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 375 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_START); >>> CID 353231: (DIVIDE_BY_ZERO) >>> In expression "vref_avg / vref_subphy_num", division by expression "vref_subphy_num" which may be zero has undefined behavior. 376 mv_ddr4_vref_tap_set(dev_num, if_id, ACCESS_TYPE_UNICAST, 377 vref_avg / vref_subphy_num, MV_DDR4_VREF_TAP_END); 378 DEBUG_CALIBRATION(DEBUG_LEVEL_INFO, ("final vref average %d\n", vref_avg / vref_subphy_num)); 379 /* run centralization again with optimal vref to update global structures */ 380 mv_ddr4_centralization(dev_num, pbs_tap_factor, c_opt_per_bus, pbs_res_per_bus, valid_win_size, 381 TX_DIR, vref_avg / vref_subphy_num, duty_cycle); ** CID 353230: Memory - corruptions (OVERRUN) ________________________________________________________________________________________________________ *** CID 353230: Memory - corruptions (OVERRUN) /drivers/marvell/mv_ddr/ddr3_debug.c: 1570 in run_xsb_test() 1564 if (write_type != 0) { 1565 CHECK_STATUS(ddr3_tip_ext_write 1566 (dev_num, if_id, addr, 1, 1567 xsb_test_table[seq])); 1568 } 1569 if (read_type != 0) { >>> CID 353230: Memory - corruptions (OVERRUN) >>> Overrunning array "data_read" of 1 4-byte elements by passing it to a function which accesses it at element index 7 (byte offset 31) using argument "1U". 1570 CHECK_STATUS(ddr3_tip_ext_read 1571 (dev_num, if_id, addr, 1, 1572 data_read)); 1573 } 1574 if ((read_type != 0) && (write_type != 0)) { 1575 ret_tmp = ** CID 353229: Memory - corruptions (OVERRUN) /plat/rockchip/common/drivers/parameter/ddr_parameter.c: 130 in ddr_region_usage_parse() ________________________________________________________________________________________________________ *** CID 353229: Memory - corruptions (OVERRUN) /plat/rockchip/common/drivers/parameter/ddr_parameter.c: 130 in ddr_region_usage_parse() 124 p.s_base[p.s_nr] = p.ns_top[i]; 125 126 /* s-region's top ends with next ns-region's base */ 127 if (i + 1 < p.ns_nr) 128 p.s_top[p.s_nr] = p.ns_base[i + 1]; 129 else >>> CID 353229: Memory - corruptions (OVERRUN) >>> Overrunning array "p.s_top" of 10 8-byte elements at element index 10 (byte offset 87) using index "p.s_nr" (which evaluates to 10). 130 p.s_top[p.s_nr] = p.boundary; 131 p.s_nr++; 132 } 133 out: 134 return p; ** CID 353228: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /lib/debugfs/devfip.c: 106 in get_entry() ________________________________________________________________________________________________________ *** CID 353228: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /lib/debugfs/devfip.c: 106 in get_entry() 100 } 101 102 if (n != sizeof(struct fip_entry)) { 103 return -1; 104 } 105 >>> CID 353228: Integer handling issues (CONSTANT_EXPRESSION_RESULT) >>> "entry->size > 9223372036854775807L" is always false regardless of the values of its operands. This occurs as the logical first operand of "||". 106 if ((entry->size > LONG_MAX) || (entry->offset_address > LONG_MAX)) { 107 return -1; 108 } 109 110 if (entry->size == 0) { 111 return 0; ** CID 353227: Parse warnings (PARSE_ERROR) /services/std_svc/spm_mm/spm_mm_xlat.c: 26 in () ________________________________________________________________________________________________________ *** CID 353227: Parse warnings (PARSE_ERROR) /services/std_svc/spm_mm/spm_mm_xlat.c: 26 in () 20 /* Place translation tables by default along with the ones used by BL31. */ 21 #ifndef PLAT_SP_IMAGE_XLAT_SECTION_NAME 22 #define PLAT_SP_IMAGE_XLAT_SECTION_NAME "xlat_table" 23 #endif 24 25 /* Allocate and initialise the translation context for the secure partitions. */ >>> CID 353227: Parse warnings (PARSE_ERROR) >>> expected a string literal 26 REGISTER_XLAT_CONTEXT2(sp, 27 PLAT_SP_IMAGE_MMAP_REGIONS, 28 PLAT_SP_IMAGE_MAX_XLAT_TABLES, 29 PLAT_VIRT_ADDR_SPACE_SIZE, PLAT_PHY_ADDR_SPACE_SIZE, 30 EL1_EL0_REGIME, PLAT_SP_IMAGE_XLAT_SECTION_NAME); 31 ** CID 353226: (VARARGS) /lib/libc/snprintf.c: 110 in snprintf() /lib/libc/snprintf.c: 93 in snprintf() /lib/libc/snprintf.c: 114 in snprintf() ________________________________________________________________________________________________________ *** CID 353226: (VARARGS) /lib/libc/snprintf.c: 110 in snprintf() 104 unum = (unsigned int)num; 105 } 106 107 unsigned_dec_print(&s, n, &chars_printed, unum); 108 break; 109 case 's': >>> CID 353226: (VARARGS) >>> Calling va_arg on va_list "args", which has not been prepared with va_start(). 110 str = va_arg(args, char *); 111 string_print(&s, n, &chars_printed, str); 112 break; 113 case 'u': 114 unum = va_arg(args, unsigned int); 115 unsigned_dec_print(&s, n, &chars_printed, unum); /lib/libc/snprintf.c: 93 in snprintf() 87 if (*fmt == '%') { 88 fmt++; 89 /* Check the format specifier. */ 90 switch (*fmt) { 91 case 'i': 92 case 'd': >>> CID 353226: (VARARGS) >>> Calling va_arg on va_list "args", which has not been prepared with va_start(). 93 num = va_arg(args, int); 94 95 if (num < 0) { 96 if (chars_printed < n) { 97 *s = '-'; 98 s++; /lib/libc/snprintf.c: 114 in snprintf() 108 break; 109 case 's': 110 str = va_arg(args, char *); 111 string_print(&s, n, &chars_printed, str); 112 break; 113 case 'u': >>> CID 353226: (VARARGS) >>> Calling va_arg on va_list "args", which has not been prepared with va_start(). 114 unum = va_arg(args, unsigned int); 115 unsigned_dec_print(&s, n, &chars_printed, unum); 116 break; 117 default: 118 /* Panic on any other format specifier. */ 119 ERROR("snprintf: specifier with ASCII code '%d' not supported.", ** CID 353225: Control flow issues (NO_EFFECT) /plat/intel/soc/common/soc/socfpga_mailbox.c: 298 in intel_mailbox_get_config_status() ________________________________________________________________________________________________________ *** CID 353225: Control flow issues (NO_EFFECT) /plat/intel/soc/common/soc/socfpga_mailbox.c: 298 in intel_mailbox_get_config_status() 292 uint32_t status, res; 293 uint32_t response[6]; 294 295 status = mailbox_send_cmd(1, cmd, NULL, 0, 0, response, 296 sizeof(response) / sizeof(response[0])); 297 >>> CID 353225: Control flow issues (NO_EFFECT) >>> This less-than-zero comparison of an unsigned value is never true. "status < 0U". 298 if (status < 0) 299 return status; 300 301 res = response[RECONFIG_STATUS_STATE]; 302 if (res && res != MBOX_CFGSTAT_STATE_CONFIG) 303 return res; ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

6 years, 1 month

1
0
0 0

Re: [TF-A] New Defects reported by Coverity Scan for ARM-software/arm-trusted-firmware

by Alexei Fedorov

Hi Varun Could you take a look at the Nvidia driver defect reported by the Coverity scan? Regards. Alexei. ________________________________ From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> on behalf of scan-admin--- via TF-A <tf-a(a)lists.trustedfirmware.org> Sent: 03 January 2020 08:19 To: tf-a(a)lists.trustedfirmware.org <tf-a(a)lists.trustedfirmware.org> Subject: [TF-A] New Defects reported by Coverity Scan for ARM-software/arm-trusted-firmware Hi, Please find the latest report on new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan. 1 new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 352832: Uninitialized variables (UNINIT) /plat/nvidia/tegra/soc/t194/drivers/mce/nvg.c: 197 in nvg_update_ccplex_gsc() ________________________________________________________________________________________________________ *** CID 352832: Uninitialized variables (UNINIT) /plat/nvidia/tegra/soc/t194/drivers/mce/nvg.c: 197 in nvg_update_ccplex_gsc() 191 ret = EINVAL; 192 } else { 193 nvg_set_request_data((uint64_t)TEGRA_NVG_CHANNEL_UPDATE_CCPLEX_GSC, 194 (uint64_t)gsc_idx); 195 } 196 >>> CID 352832: Uninitialized variables (UNINIT) >>> Using uninitialized value "ret". 197 return ret; 198 } 199 200 /* 201 * Cache clean operation for all CCPLEX caches. 202 */ ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

6 years, 1 month

1
0
0 0

New Defects reported by Coverity Scan for ARM-software/arm-trusted-firmware

by scan-admin＠coverity.com

Hi, Please find the latest report on new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan. 1 new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 352832: Uninitialized variables (UNINIT) /plat/nvidia/tegra/soc/t194/drivers/mce/nvg.c: 197 in nvg_update_ccplex_gsc() ________________________________________________________________________________________________________ *** CID 352832: Uninitialized variables (UNINIT) /plat/nvidia/tegra/soc/t194/drivers/mce/nvg.c: 197 in nvg_update_ccplex_gsc() 191 ret = EINVAL; 192 } else { 193 nvg_set_request_data((uint64_t)TEGRA_NVG_CHANNEL_UPDATE_CCPLEX_GSC, 194 (uint64_t)gsc_idx); 195 } 196 >>> CID 352832: Uninitialized variables (UNINIT) >>> Using uninitialized value "ret". 197 return ret; 198 } 199 200 /* 201 * Cache clean operation for all CCPLEX caches. 202 */ ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

6 years, 1 month

1
0
0 0

Re: [TF-A] [Question] Why are the log related macros defined in multiple of 10's

by Olivier Deprez

Hi Sheetal, Your remark/question makes sense. The original requirement is described here: https://github.com/ARM-software/tf-issues/issues/232 We may get further update from Dan, but I sense this was to provision for intermediate log levels (as stated by Sandrine on phabricator). Will you be able to submit a patch for this? Regards, Olivier. ________________________________________ From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> on behalf of Sheetal Tigadoli via TF-A <tf-a(a)lists.trustedfirmware.org> Sent: 20 December 2019 15:48 To: tf-a(a)lists.trustedfirmware.org Subject: [TF-A] [Question] Why are the log related macros defined in multiple of 10's Hi All, I wanted to understand the reason behind defining LOG_LEVEL_* macros as multiple of 10's, also same goes with LOG_MARKER_* macros. If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and division operation when printing the msgs to console. Please let me your thoughts on this. Link to this question on Phabricator - https://developer.trustedfirmware.org/T624 Thanks Sheetal -- TF-A mailing list TF-A(a)lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a

6 years, 1 month

2
1
0 0

[Question] Why are the log related macros defined in multiple of 10's

by Sheetal Tigadoli

Hi All, I wanted to understand the reason behind defining LOG_LEVEL_* macros as multiple of 10's, also same goes with LOG_MARKER_* macros. If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and division operation when printing the msgs to console. Please let me your thoughts on this. Link to this question on Phabricator - https://developer.trustedfirmware.org/T624 Thanks Sheetal

6 years, 1 month

1
0
0 0

[Question] Why are the log related macros defined in multiple of 10's

by Sheetal Tigadoli

Hi All, I wanted to understand the reason behind defining LOG_LEVEL_* macros as multiple of 10's, also same goes with LOG_MARKER_* macros. If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and division operation when printing the msgs to console. Please let me your thoughts on this. Link to this question on Phabricator - https://developer.trustedfirmware.org/T624 Thanks Sheetal

6 years, 1 month

1
0
0 0

2026

2025

2024

2023

2022

2021

2020

2019

2018

TF-A