LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] exfat: tighten down num_fats check
@ 2020-02-14 23:28 ` Valdis Klētnieks
  2020-02-17  0:37   ` Namjae Jeon
  0 siblings, 1 reply; 3+ messages in thread
From: Valdis Klētnieks @ 2020-02-14 23:28 UTC (permalink / raw)
  To: Namjae Jeon, Sasha Levin
  Cc: linux-fsdevel, linux-kernel, gregkh, hch, sj1557.seo, pali.rohar,
	arnd, namjae.jeon, viro

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

Change the test for num_fats from != 0 to a check for specifically 1.

Although it's theoretically possible that num_fats == 2 for a TexFAT volume (or
an implementation that doesn't do the full TexFAT but does support 2 FAT
tables), the rest of the code doesn't currently DTRT if it's 2 (in particular,
not handling the case of ActiveFat pointing at the second FAT area), so we'll
disallow that as well, as well as dealing with corrupted images that have a
trash non-zero value.

Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>

--- a/fs/exfat/super.c	2020-02-14 17:45:02.262274632 -0500
+++ b/fs/exfat/super.c	2020-02-14 17:46:37.200343723 -0500
@@ -450,7 +450,7 @@ static int __exfat_fill_super(struct sup
 	}

 	p_bpb = (struct pbr64 *)p_pbr;
-	if (!p_bpb->bsx.num_fats) {
+	if (p_bpb->bsx.num_fats  != 1) {
 		exfat_msg(sb, KERN_ERR, "bogus number of FAT structure");
 		ret = -EINVAL;
 		goto free_bh;






[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] exfat: tighten down num_fats check
  2020-02-14 23:28 ` [PATCH] exfat: tighten down num_fats check Valdis Klētnieks
@ 2020-02-17  0:37   ` Namjae Jeon
  2020-02-17  1:41     ` Valdis Klētnieks
  0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2020-02-17  0:37 UTC (permalink / raw)
  To: \'Valdis Klētnieks\
  Cc: linux-fsdevel, linux-kernel, gregkh, hch, sj1557.seo, pali.rohar,
	arnd, viro, Namjae Jeon, Sasha Levin

> Change the test for num_fats from != 0 to a check for specifically 1.
> 
> Although it's theoretically possible that num_fats == 2 for a TexFAT
> volume (or an implementation that doesn't do the full TexFAT but does
> support 2 FAT tables), the rest of the code doesn't currently DTRT if it's
> 2 (in particular, not handling the case of ActiveFat pointing at the
> second FAT area), so we'll disallow that as well, as well as dealing with
> corrupted images that have a trash non-zero value.
> 
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> 
> --- a/fs/exfat/super.c	2020-02-14 17:45:02.262274632 -0500
> +++ b/fs/exfat/super.c	2020-02-14 17:46:37.200343723 -0500
> @@ -450,7 +450,7 @@ static int __exfat_fill_super(struct sup
>  	}
> 
>  	p_bpb = (struct pbr64 *)p_pbr;
> -	if (!p_bpb->bsx.num_fats) {
> +	if (p_bpb->bsx.num_fats  != 1) {
>  		exfat_msg(sb, KERN_ERR, "bogus number of FAT structure");
Could you please update error message for the reason why num_fats is allowed
only 1?
>  		ret = -EINVAL;
>  		goto free_bh;
Let's remove exfat_mirror_bh(), FAT2_start_sector variable and the below
related codes together.

sbi->FAT2_start_sector = p_bpb->bsx.num_fats == 1 ?
                sbi->FAT1_start_sector :
                        sbi->FAT1_start_sector + sbi->num_FAT_sectors;

Thanks for your patch!
> 
> 
> 
> 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] exfat: tighten down num_fats check
  2020-02-17  0:37   ` Namjae Jeon
@ 2020-02-17  1:41     ` Valdis Klētnieks
  0 siblings, 0 replies; 3+ messages in thread
From: Valdis Klētnieks @ 2020-02-17  1:41 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, gregkh, hch, sj1557.seo, pali.rohar,
	arnd, viro, Namjae Jeon, Sasha Levin

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

On Mon, 17 Feb 2020 09:37:55 +0900, "Namjae Jeon" said:

> Could you please update error message for the reason why num_fats is allowed
> only 1?

Sure.. No problem..

> >  		ret = -EINVAL;
> >  		goto free_bh;
> Let's remove exfat_mirror_bh(), FAT2_start_sector variable and the below
> related codes together.
>
> sbi->FAT2_start_sector = p_bpb->bsx.num_fats == 1 ?
>                 sbi->FAT1_start_sector :
>                         sbi->FAT1_start_sector + sbi->num_FAT_sectors;

You might want to hold off on that part for a bit - I've asked Sasha Levin for
input on what exactly Windows does with this, and Pali has a not-obviously-wrong
suggestion on using the second FAT table.  The code tracking FAT2_start_sector
looks OK - what would be missing is doing a similar versioning on the FAT
the rest of the code references.

We may end up heaving that code over the side in the end, but let's make
sure we're doing it with more information in hand....


[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200214232853epcas1p241e47cdc4e0b9b5c603cc6eaa6182360@epcas1p2.samsung.com>
2020-02-14 23:28 ` [PATCH] exfat: tighten down num_fats check Valdis Klētnieks
2020-02-17  0:37   ` Namjae Jeon
2020-02-17  1:41     ` Valdis Klētnieks

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git