Re: BUG: Out of bounds read in hci_le_ext_adv_report_evt()
Hi, Den mån 22 mars 2021 kl 00:01 skrev Luiz Augusto von Dentz : > Or we do something like > https://lore.kernel.org/linux-bluetooth/20201024002251.1389267-1-luiz.de...@gmail.com/, > that said the reason we didn't applied my patches was that the > controller would be the one generating invalid data, but it seems you > are reproducing with vhci controller which is only used for emulating > a controller and requires root privileges so it is unlikely these > conditions would happens with hardware itself, in the other hand as > there seems to be more and more reports using vhci to emulate broken > events it perhaps more productive to introduce proper checks for all > events so we don't have to deal with more reports like this in the > future. Keep in mind that when using the H4 uart protocol without any error correction (as H5 has), it is possible that random bit errors occur on the wire. I wouldn't like my kernel to crash due to this. Bit errors happen all the time on RPi 4 for example at the default baud rate if you just do some heavy stress testing, or use an application that transfers a lot of data over Bluetooth. /Emil
Re: [PATCH] ubifs: Add support for zstd compression.
Hi, Den fre 7 juni 2019 kl 22:49 skrev Richard Weinberger : > > - Ursprüngliche Mail - > > Von: "Emil Lenngren" > > An: "richard" > > CC: "linux-mtd" , "Sebastian Andrzej > > Siewior" , "linux-kernel" > > , "Michele Dionisio" > > > > Gesendet: Freitag, 7. Juni 2019 22:27:09 > > Betreff: Re: [PATCH] ubifs: Add support for zstd compression. > >> So I'm not sure what is the best choice for the default filesystem. > > > > My idea was at the end, i.e. it will only be used when LZO and ZLIB > > are not selected to be included for UBIFS, for example when someone > > compiles a minimal kernel who knows exactly which compression > > algorithms will be used on that system. > > BTW: you can always select the compressor using the compr= mount option. > Also for the default filesystem. Yep that's what I'm using while I'm testing. > Putting it at the end does not harm but IMHO the use is little. > But for the sake of completes, I agree with you. Can you send a follow-up > patch? Ok > > > I did a single test today and compared lzo and zstd and on that test > > lzo had faster decompression speed but resulted in larger space. I'll > > do more tests later. > > Can you please share more details? I'm interested what CPU this was. ARM Cortex-A7. The kernel is compiled with gcc 7.3.1. Next week I'll test some more. I have a question about how the decompression is done while reading. When a large file is read from the filesystem (assuming not in any cache), is it the case that first a chunk is read from flash, that chunk is then decompressed, later next chunk is read from flash, that one is then decompressed and so on, or can the decompression be done in parallel while reading the next chunk from flash? /Emil
Re: [PATCH] ubifs: Add support for zstd compression.
Hi, Den fre 7 juni 2019 kl 22:09 skrev Richard Weinberger : > > Emil, > > - Ursprüngliche Mail - > > In fs/ubifs/sb.c we have > > > > static int get_default_compressor(struct ubifs_info *c) > > { > >if (ubifs_compr_present(c, UBIFS_COMPR_LZO)) > >return UBIFS_COMPR_LZO; > > > >if (ubifs_compr_present(c, UBIFS_COMPR_ZLIB)) > >return UBIFS_COMPR_ZLIB; > > > >return UBIFS_COMPR_NONE; > > } > > > > Maybe add an entry for zstd here as well? > > Where would you add it? If we add it after UBIFS_COMPR_ZLIB, > it will effectively never get selected, unless someone builds a kernel > without lzo and zlib but zstd. > If we add it before UBIFS_COMPR_ZLIB, it will be used always and users > end up with unreadable files if they reboot to an older kernel. > Please note that we didn't raise the UBIFS format version for zstd. > > So I'm not sure what is the best choice for the default filesystem. My idea was at the end, i.e. it will only be used when LZO and ZLIB are not selected to be included for UBIFS, for example when someone compiles a minimal kernel who knows exactly which compression algorithms will be used on that system. I guess that's the same reason why zlib exists at all and is placed after lzo. But of course the other positions also have their pros. If we perform more benchmarks speed/size and conclude that zstd outperforms zlib for all aspects, then maybe placing it between lzo and zlib could be an option, but as you say we should avoid breaking compatibility with older systems. I did a single test today and compared lzo and zstd and on that test lzo had faster decompression speed but resulted in larger space. I'll do more tests later. /Emil
Re: [PATCH] ubifs: Add support for zstd compression.
Hello, Den ons 15 maj 2019 kl 23:03 skrev Richard Weinberger : > > From: Michele Dionisio > > zstd shows a good compression rate and is faster than lzo, > also on slow ARM cores. > > Cc: Sebastian Andrzej Siewior > Signed-off-by: Michele Dionisio > [rw: rewrote commit message] > Signed-off-by: Richard Weinberger > --- > fs/ubifs/Kconfig | 10 ++ > fs/ubifs/compress.c| 27 ++- > fs/ubifs/super.c | 2 ++ > fs/ubifs/ubifs-media.h | 2 ++ > 4 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig > index 9da2f135121b..8d84d2ed096d 100644 > --- a/fs/ubifs/Kconfig > +++ b/fs/ubifs/Kconfig > @@ -5,8 +5,10 @@ config UBIFS_FS > select CRYPTO if UBIFS_FS_ADVANCED_COMPR > select CRYPTO if UBIFS_FS_LZO > select CRYPTO if UBIFS_FS_ZLIB > + select CRYPTO if UBIFS_FS_ZSTD > select CRYPTO_LZO if UBIFS_FS_LZO > select CRYPTO_DEFLATE if UBIFS_FS_ZLIB > + select CRYPTO_ZSTD if UBIFS_FS_ZSTD > select CRYPTO_HASH_INFO > select UBIFS_FS_XATTR if FS_ENCRYPTION > depends on MTD_UBI > @@ -37,6 +39,14 @@ config UBIFS_FS_ZLIB > help > Zlib compresses better than LZO but it is slower. Say 'Y' if unsure. > > +config UBIFS_FS_ZSTD > + bool "ZSTD compression support" if UBIFS_FS_ADVANCED_COMPR > + depends on UBIFS_FS > + default y > + help > + ZSTD compresses is a big win in speed over Zlib and > + in compression ratio over LZO. Say 'Y' if unsure. > + > config UBIFS_ATIME_SUPPORT > bool "Access time support" > default n > diff --git a/fs/ubifs/compress.c b/fs/ubifs/compress.c > index 565cb56d7225..89183aeeeb7a 100644 > --- a/fs/ubifs/compress.c > +++ b/fs/ubifs/compress.c > @@ -71,6 +71,24 @@ static struct ubifs_compressor zlib_compr = { > }; > #endif > > +#ifdef CONFIG_UBIFS_FS_ZSTD > +static DEFINE_MUTEX(zstd_enc_mutex); > +static DEFINE_MUTEX(zstd_dec_mutex); > + > +static struct ubifs_compressor zstd_compr = { > + .compr_type = UBIFS_COMPR_ZSTD, > + .comp_mutex = &zstd_enc_mutex, > + .decomp_mutex = &zstd_dec_mutex, > + .name = "zstd", > + .capi_name = "zstd", > +}; > +#else > +static struct ubifs_compressor zstd_compr = { > + .compr_type = UBIFS_COMPR_ZSTD, > + .name = "zstd", > +}; > +#endif > + > /* All UBIFS compressors */ > struct ubifs_compressor *ubifs_compressors[UBIFS_COMPR_TYPES_CNT]; > > @@ -228,13 +246,19 @@ int __init ubifs_compressors_init(void) > if (err) > return err; > > - err = compr_init(&zlib_compr); > + err = compr_init(&zstd_compr); > if (err) > goto out_lzo; > > + err = compr_init(&zlib_compr); > + if (err) > + goto out_zstd; > + > ubifs_compressors[UBIFS_COMPR_NONE] = &none_compr; > return 0; > > +out_zstd: > + compr_exit(&zstd_compr); > out_lzo: > compr_exit(&lzo_compr); > return err; > @@ -247,4 +271,5 @@ void ubifs_compressors_exit(void) > { > compr_exit(&lzo_compr); > compr_exit(&zlib_compr); > + compr_exit(&zstd_compr); > } > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > index 04b8ecfd3470..ea8615261936 100644 > --- a/fs/ubifs/super.c > +++ b/fs/ubifs/super.c > @@ -1055,6 +1055,8 @@ static int ubifs_parse_options(struct ubifs_info *c, > char *options, > c->mount_opts.compr_type = UBIFS_COMPR_LZO; > else if (!strcmp(name, "zlib")) > c->mount_opts.compr_type = UBIFS_COMPR_ZLIB; > + else if (!strcmp(name, "zstd")) > + c->mount_opts.compr_type = UBIFS_COMPR_ZSTD; > else { > ubifs_err(c, "unknown compressor \"%s\"", > name); //FIXME: is c ready? > kfree(name); > diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h > index 8b7c1844014f..697b1b89066a 100644 > --- a/fs/ubifs/ubifs-media.h > +++ b/fs/ubifs/ubifs-media.h > @@ -348,12 +348,14 @@ enum { > * UBIFS_COMPR_NONE: no compression > * UBIFS_COMPR_LZO: LZO compression > * UBIFS_COMPR_ZLIB: ZLIB compression > + * UBIFS_COMPR_ZSTD: ZSTD compression > * UBIFS_COMPR_TYPES_CNT: count of supported compression types > */ > enum { > UBIFS_COMPR_NONE, > UBIFS_COMPR_LZO, > UBIFS_COMPR_ZLIB, > + UBIFS_COMPR_ZSTD, > UBIFS_COMPR_TYPES_CNT, > }; > > -- > 2.16.4 In fs/ubifs/sb.c we have static int get_default_compressor(struct ubifs_info *c) { if (ubifs_compr_present(c, UBIFS_COMPR_LZO)) return UBIFS_COMPR_LZO; if (ubifs_compr_present(c, UBIFS_COMPR_ZLIB)) return UBIFS_COMPR_ZLIB; return UBIFS_COMPR_NONE; } Maybe add an entry for zstd here as well? /Emil
Re: [PATCH] pre-shared passcode: secure pairing for "no keyboard, no display" devices
Hi, Den fre 15 feb. 2019 kl 12:46 skrev Pavel Machek : > > Hi! > > > > Currently, "no keyboard, no display" devices can be paired, but > > > pairing is not secure against active attacker. > > > > > > Can we do better? Not for the first pairing; but for the next ones -- > > > yes, I believe we can. > > > > > > BLE device in this case has internal storage, and Linux running > > > there. From factory, random 6-digit number is stored in the > > > flash. Legitimate user knows the number, and system is manipulated so > > > that pairing passkey will be this pre-shared passkey. After pairing, > > > user is allowed to change it. > > > > > > [Or maybe passkey is 00 from the factory; this is still win for > > > the user, as long as he can change the key to something random in a > > > secure cave.] > > > > > > Fortunately, kernel support for this is rather easy; patch is attached > > > below. > > > > > > Does someone see a security issue with proposal above? > > > > Assuming "LE Secure Connections" is used, the protocol is only secure > > if the passkey is not reused. A 6 digit passkey means 20 bits. The > > protocol is performed in 20 steps, where one bit is compared and > > revealed in every step. This means the attacker will know for every > > attempt the first bit that is incorrect in the attempted passkey. If > > the passkey is reused, the attacker can just try the same passkey with > > the incorrect bit flipped. In average it takes 10 attempts to crack > > the key and maximum 20 attempts. Hence, a static key doesn't really > > add much security compared to Just Works and might give a false sense > > of security. > > Thanks a lot for quick reply. And no, that is not good. > > Just Works basically means that there's no security at all, anyone > within range can connect when owner is not around, and do whatever > they want. > > Is there some common way this is solved? > > We do have pre-shared passkey, if only 20 bits. If we set > passkey = sha( pre-shared passkey + pairing_attempt++ ), would we get > some kind of meaningful security? Perhaps with limiting pairing > attempt to say 10 a day? I see two issues with that approach. First, the user needs to know the correct counter of pairing_attempt (how?) and perform the sha calculation and map that to a passkey. Second, if you eavesdrop a pairing attempt that succeeds, you can just bruteforce sha over all possible pre-shared passkeys (1 million if you have 6 digits) times the number of different pairing_attempt counters you want to test, and match that with the recorded pairing messages. That shouldn't take more than a few seconds on a normal PC. What people really do that wants proper security over BLE when they need to use static passkeys, is using a PAKE algorithm in the application layer to establish a session key, and then use some AEAD cipher with that session key also in the application layer (or potentially start BLE encryption using that session key as LTK, but I haven't seen that). Unencrypted BLE is just used then as a transport layer. /Emil
Re: [PATCH] pre-shared passcode: secure pairing for "no keyboard, no display" devices
Hi Pavel, Den tors 14 feb. 2019 kl 14:59 skrev Pavel Machek : > > Hi! > > Currently, "no keyboard, no display" devices can be paired, but > pairing is not secure against active attacker. > > Can we do better? Not for the first pairing; but for the next ones -- > yes, I believe we can. > > BLE device in this case has internal storage, and Linux running > there. From factory, random 6-digit number is stored in the > flash. Legitimate user knows the number, and system is manipulated so > that pairing passkey will be this pre-shared passkey. After pairing, > user is allowed to change it. > > [Or maybe passkey is 00 from the factory; this is still win for > the user, as long as he can change the key to something random in a > secure cave.] > > Fortunately, kernel support for this is rather easy; patch is attached > below. > > Does someone see a security issue with proposal above? Assuming "LE Secure Connections" is used, the protocol is only secure if the passkey is not reused. A 6 digit passkey means 20 bits. The protocol is performed in 20 steps, where one bit is compared and revealed in every step. This means the attacker will know for every attempt the first bit that is incorrect in the attempted passkey. If the passkey is reused, the attacker can just try the same passkey with the incorrect bit flipped. In average it takes 10 attempts to crack the key and maximum 20 attempts. Hence, a static key doesn't really add much security compared to Just Works and might give a false sense of security. A static key for Legacy Pairing has comparable security as using a random key. The security is kind of the same as logging in to a website over unencrypted HTTP, assuming the static key is stored on the peripheral and the user enters the passkey on the central. In the case where the passkey is entered by the user on the peripheral instead of the central, the protocol is completely broken (https://eprint.iacr.org/2013/309.pdf). /Emil
Re: [PATCH 2/2] mtd: spinand: micron: Support for all Micron SPI NAND flashes
Hi, Den mån 4 feb. 2019 kl 12:18 skrev Shivamurthy Shastri (sshivamurthy) : > > Driver is redesigned using parameter page to support all the Micron > SPI NAND flashes. > > Parameter page of Micron flashes is similar to ONFI parameter table and > functionality is same, so copied some of the common functions like crc16 > and bit_wise_majority from nand_onfi.c. > > This driver is tested using MT29F2G01ABXGD, MT29F4G01ABXFD, MT29F8G01ADXFD, > MT29F1G01ABXFD. > > -static const struct spinand_info micron_spinand_table[] = { > - SPINAND_INFO("MT29F2G01ABAGD", 0x24, > -NAND_MEMORG(1, 2048, 128, 64, 2048, 2, 1, 1), > + deviceinfo.memorg.eraseblocks_per_lun = > + params->blocks_per_lun * params->lun_count; > + deviceinfo.memorg.planes_per_lun = params->lun_count; > + deviceinfo.memorg.luns_per_target = 1; > + deviceinfo.memorg.ntargets = 1; > + __le32 blocks_per_lun; > + u8 lun_count; > + u8 addr_cycles; > + u8 bits_per_cell; > + __le16 bb_per_lun; I have a question about the lun_count. As it is now, the planes_per_lun parameter is initialized to 2 in NAND_MEMORG. In your patch, it is instead initialized from the "lun_count" property from the parameter table. But I looked at a datasheet I found by a simple Google search (https://www.google.se/search?q=micron+nand+spi+datasheet), the first hit is to the 1 Gb flash MT29F1G01AAADD. That device clearly has two planes per lun (you need the "plane select" bit in the requests), but still, according to the parameter page data structure, byte 100, Number of logical units is set to 01h. Also, the "blocks per lun" count, which is called "blocks per unit" is 1024, which should be 512 if this parameter really meant "blocks per plane" and the calculation in the patch was correct. As a reference, the 2 Gb version of the Macronix flash (http://www.macronix.com/Lists/Datasheet/Attachments/6866/MX35LF2GE4AB,%203V,%202Gb,%20v1.5.pdf), also has two planes per lun. It also sets byte 100, Number of logical units to 01h. So what I'm wondering is of course if this parameter is the correct one to use for planes_per_lun. I tried to locate the correct "planes per lun" parameter in the table, but didn't find anyone. Maybe it's the unfortunate fact that "planes per lun" isn't exposed in the parameter table? /Emil
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important
2017-06-11 22:48 GMT+02:00 Emmanuel Grumbach : > On Sun, Jun 11, 2017 at 4:36 PM, Kees Cook wrote: >> >> On Sun, Jun 11, 2017 at 1:13 AM, Kalle Valo wrote: >> > "Jason A. Donenfeld" writes: >> > >> >> Whenever you're comparing two MACs, it's important to do this using >> >> crypto_memneq instead of memcmp. With memcmp, you leak timing information, >> >> which could then be used to iteratively forge a MAC. >> > >> > Do you have any pointers where I could learn more about this? >> >> While not using C specifically, this talks about the problem generally: >> https://www.chosenplaintext.ca/articles/beginners-guide-constant-time-cryptography.html >> > > Sorry for the stupid question, but the MAC address is in plaintext in > the air anyway or easily accessible via user space tools. I fail to > see what it is so secret about a MAC address in that code where that > same MAC address is accessible via myriads of ways. I think you're mixing up Media Access Control (MAC) addresses with Message Authentication Code (MAC). The second one is a cryptographic signature of a message.