Re: [PATCH] hfs/hfsplus: Clean up unused variables in bnode.c
On Sat, Oct 14, 2017 at 11:32:26AM +0100, Christos Gkekas wrote:
> Delete variables 'tree' and 'sb', which are set but never used.
>
> Signed-off-by: Christos Gkekas
Looks good. If it helps you can add:
Reviewed-by: Ernesto A. Fernández
> ---
> fs/hfs/bnode.c | 4
> fs/hfsplus/bnode.c | 4
> 2 files changed, 8 deletions(-)
>
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index d77d844..8d618c9 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -97,13 +97,11 @@ void hfs_bnode_clear(struct hfs_bnode *node, int off, int
> len)
> void hfs_bnode_copy(struct hfs_bnode *dst_node, int dst,
> struct hfs_bnode *src_node, int src, int len)
> {
> - struct hfs_btree *tree;
> struct page *src_page, *dst_page;
>
> hfs_dbg(BNODE_MOD, "copybytes: %u,%u,%u\n", dst, src, len);
> if (!len)
> return;
> - tree = src_node->tree;
> src += src_node->page_offset;
> dst += dst_node->page_offset;
> src_page = src_node->page[0];
> @@ -236,7 +234,6 @@ struct hfs_bnode *hfs_bnode_findhash(struct hfs_btree
> *tree, u32 cnid)
>
> static struct hfs_bnode *__hfs_bnode_create(struct hfs_btree *tree, u32 cnid)
> {
> - struct super_block *sb;
> struct hfs_bnode *node, *node2;
> struct address_space *mapping;
> struct page *page;
> @@ -248,7 +245,6 @@ static struct hfs_bnode *__hfs_bnode_create(struct
> hfs_btree *tree, u32 cnid)
> return NULL;
> }
>
> - sb = tree->inode->i_sb;
> size = sizeof(struct hfs_bnode) + tree->pages_per_bnode *
> sizeof(struct page *);
> node = kzalloc(size, GFP_KERNEL);
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index ce014ce..9cd8b42 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -126,14 +126,12 @@ void hfs_bnode_clear(struct hfs_bnode *node, int off,
> int len)
> void hfs_bnode_copy(struct hfs_bnode *dst_node, int dst,
> struct hfs_bnode *src_node, int src, int len)
> {
> - struct hfs_btree *tree;
> struct page **src_page, **dst_page;
> int l;
>
> hfs_dbg(BNODE_MOD, "copybytes: %u,%u,%u\n", dst, src, len);
> if (!len)
> return;
> - tree = src_node->tree;
> src += src_node->page_offset;
> dst += dst_node->page_offset;
> src_page = src_node->page + (src >> PAGE_SHIFT);
> @@ -400,7 +398,6 @@ struct hfs_bnode *hfs_bnode_findhash(struct hfs_btree
> *tree, u32 cnid)
>
> static struct hfs_bnode *__hfs_bnode_create(struct hfs_btree *tree, u32 cnid)
> {
> - struct super_block *sb;
> struct hfs_bnode *node, *node2;
> struct address_space *mapping;
> struct page *page;
> @@ -413,7 +410,6 @@ static struct hfs_bnode *__hfs_bnode_create(struct
> hfs_btree *tree, u32 cnid)
> return NULL;
> }
>
> - sb = tree->inode->i_sb;
> size = sizeof(struct hfs_bnode) + tree->pages_per_bnode *
> sizeof(struct page *);
> node = kzalloc(size, GFP_KERNEL);
> --
> 2.7.4
>
Re: [PATCH] hfsplus: don't return 0 when fill_super() failed
On Tue, May 15, 2018 at 07:08:24PM +0900, Tetsuo Handa wrote:
> From f78a5fe168290cb9e009f4d907d04b5bfe277831 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa
> Date: Tue, 15 May 2018 11:38:38 +0900
> Subject: [PATCH] hfsplus: don't return 0 when fill_super() failed
>
> syzbot is reporting NULL pointer dereference at mount_fs() [1].
> This is because hfsplus_fill_super() is by error returning 0 when
> hfsplus_fill_super() detected invalid filesystem image, and mount_bdev()
> is returning NULL because dget(s->s_root) == NULL if s->s_root == NULL,
> and mount_fs() is accessing root->d_sb because IS_ERR(root) == false
> if root == NULL. Fix this by returning -EINVAL when hfsplus_fill_super()
> detected invalid filesystem image.
>
> [1]
> https://syzkaller.appspot.com/bug?id=21acb6850cecbc960c927229e597158cf35f33d0
>
> Signed-off-by: Tetsuo Handa
> Reported-by: syzbot
> Cc: Al Viro
It's been too long. I think I should give up on my patch. Maybe a review
can help your version get merged.
Reviewed-by: Ernesto A. Fernández
> ---
> fs/hfsplus/super.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 513c357..9e690ae 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -524,8 +524,10 @@ static int hfsplus_fill_super(struct super_block *sb,
> void *data, int silent)
> goto out_put_root;
> if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
> hfs_find_exit(&fd);
> - if (entry.type != cpu_to_be16(HFSPLUS_FOLDER))
> + if (entry.type != cpu_to_be16(HFSPLUS_FOLDER)) {
> + err = -EINVAL;
> goto out_put_root;
> + }
> inode = hfsplus_iget(sb, be32_to_cpu(entry.folder.id));
> if (IS_ERR(inode)) {
> err = PTR_ERR(inode);
> --
> 1.8.3.1
>
>
Re: Mounting corrupted HFS+ causes kernel NULL pointer dereference
Hi again: A patch for your original report has already been added to the -mm tree. On Tue, Jun 12, 2018 at 09:43:26PM +0300, Anatoly Trosinenko wrote: > Now, when mounting the attached hfsplus_16mb_segv to /mnt and > performing `dd if=/dev/zero of=/mnt/xyz bs=567879 count=1` I get > > [1.646451] BUG: unable to handle kernel NULL pointer dereference > at 0043 I just sent you a patch for this second report. It's really simple, so it would be great if you could take a look at it and review it yourself. Otherwise it's not very likely to get picked up. Thanks, Ernest
Re: Mounting corrupted HFS+ causes kernel NULL pointer dereference
On Fri, Jun 29, 2018 at 03:45:43PM -0300, Ernesto A. Fernández wrote: > Hi again: > > A patch for your original report has already been added to the -mm tree. > > On Tue, Jun 12, 2018 at 09:43:26PM +0300, Anatoly Trosinenko wrote: > > Now, when mounting the attached hfsplus_16mb_segv to /mnt and > > performing `dd if=/dev/zero of=/mnt/xyz bs=567879 count=1` I get > > > > [1.646451] BUG: unable to handle kernel NULL pointer dereference > > at 0043 > > I just sent you a patch for this second report. It's really simple, so > it would be great if you could take a look at it and review it yourself. > Otherwise it's not very likely to get picked up. Never mind, it got picked up already. > > Thanks, > Ernest
Re: Mounting corrupted HFS+ causes kernel NULL pointer dereference
On Tue, Jun 12, 2018 at 09:43:26PM +0300, Anatoly Trosinenko wrote: > And when I mount hfsplus_16mb_hang and perform `echo > /mnt/xyz`, it hangs. I just sent you a patch for this final report. Let me know if it works for you.
Re: Mounting corrupted HFS+ causes kernel NULL pointer dereference
On Tue, Jul 10, 2018 at 08:28:37PM +0300, Anatoly Trosinenko wrote: > Thank you, > > When applied this single patch on v4.18-rc4 and performed "echo > > /mnt/xyz" on hfsplus_16mb_hang image, I get about 14 pairs of lines > > hfsplus: unable to mark blocks free: error -5 > hfsplus: can't free extent > > Then `echo` exits with "No space left on device" error. Truncation does not return error codes in hfsplus, hence this weird "No space left" that comes from somewhere else. This should be fixed, but it's not as big an issue as the deadlock. Filesystems usually don't need to worry about protecting a crafted image from acting weird and causing damage to itself. >Then it > permits to perform `rm /mnt/xyz` and on `echo > /mnt/1` it responds > with no space left on device (but file *is* created and is cattable). > I don't know what is safer, but now it doesn't deadlock. :) Maybe it > is even worth to remount FS r/o, I don't know. (Please excuse me for > speculations) It's not strange that the /mnt/1 file could be created but not written to, since the first operation doesn't usually require allocating blocks. > > Thanks, > Anatoly OK, I'll take a look at the truncation error codes as soon as I'm done with the other deadlocks I found. It could take a while. Thanks for the testing. Ernest > пн, 9 июл. 2018 г. в 23:35, Ernesto A. Fernández > : > > > > On Tue, Jun 12, 2018 at 09:43:26PM +0300, Anatoly Trosinenko wrote: > > > And when I mount hfsplus_16mb_hang and perform `echo > /mnt/xyz`, it > > > hangs. > > > > I just sent you a patch for this final report. Let me know if it works > > for you.
Re: Mounting corrupted HFS+ causes kernel NULL pointer dereference
Hi, thank you for your report. On Sun, Jun 03, 2018 at 06:52:19PM +0300, Anatoly Trosinenko wrote: > How to reproduce: > 1. Take kernel source v4.17-rc7 > 2. Compile it with the config attached > 3. Unpack and mount the attached FS image as hfsplus. We are aware of this issue and I've sent some patches [1][2]. It's hard to get reviewers interested in hfsplus, so I don't know when it will be fixed. [1] https://www.spinics.net/lists/linux-fsdevel/msg125241.html [2] https://www.spinics.net/lists/linux-fsdevel/msg126499.html
Re: Mounting corrupted HFS+ causes kernel NULL pointer dereference
R09: > 000a > [1.650863] R10: 01b6 R11: 0246 R12: > 7ff996f60010 > [1.650946] R13: 0008aa47 R14: 7ff996f60010 R15: > > [1.651058] Code: 39 5a 68 77 ce 48 89 ef 5b 5d e9 03 c7 ef ff 0f > 1f 00 48 85 ff 74 04 3e ff 47 48 f3 c3 0f 1f 44 00 00 48 85 ff 74 5b > 41 54 55 53 <8b> 47 48 48 8b 2f 85 c0 0f 84 89 00 00 00 49 89 fc 48 8d > 75 6c > [1.651492] RIP: hfsplus_bnode_put+0x9/0xc0 RSP: b750409b7a58 > [1.651583] CR2: 0043 > [1.651851] ---[ end trace d164982d45c0eb53 ]--- > > (full log attached) > > And when I mount hfsplus_16mb_hang and perform `echo > /mnt/xyz`, it hangs. > > PS: Please excuse me, if these patches just became slightly outdated > and I didn't managed to apply them properly. > пт, 8 июн. 2018 г. в 18:25, Pavel Machek : > > > > On Sun 2018-06-03 15:49:56, Ernesto A. Fernández wrote: > > 1;2802;0c> Hi, thank you for your report. > > > > > > On Sun, Jun 03, 2018 at 06:52:19PM +0300, Anatoly Trosinenko wrote: > > > > How to reproduce: > > > > 1. Take kernel source v4.17-rc7 > > > > 2. Compile it with the config attached > > > > 3. Unpack and mount the attached FS image as hfsplus. > > > > > > We are aware of this issue and I've sent some patches [1][2]. It's hard > > > to get reviewers interested in hfsplus, so I don't know when it will be > > > fixed. > > > > I guess Anatoly can still test the patches, and add Tested-by tags if > > they help. No guarantees, but that may make it easier to get the patches > > merged. > > > > Thanks, > > Pavel > > > > > [1] https://www.spinics.net/lists/linux-fsdevel/msg125241.html > > > [2] https://www.spinics.net/lists/linux-fsdevel/msg126499.html > > > > -- > > (english) http://www.livejournal.com/~pavelmachek > > (cesky, pictures) > > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html > > > > -- > С уважением, > Анатолий Тросиненко > e-mail: anatoly.trosine...@gmail.com > q[0.00] Linux version 4.17.0+ (trosinenko@trosinenko-pc) (gcc version > 7.3.0 (Ubuntu 7.3.0-16ubuntu3)) #1 SMP Tue Jun 12 21:03:04 MSK 2018 > [0.00] Command line: console=ttyS0 > [0.00] x86/fpu: x87 FPU will use FXSAVE > [0.00] e820: BIOS-provided physical RAM map: > [0.00] BIOS-e820: [mem 0x-0x0009fbff] usable > [0.00] BIOS-e820: [mem 0x0009fc00-0x0009] reserved > [0.00] BIOS-e820: [mem 0x000f-0x000f] reserved > [0.00] BIOS-e820: [mem 0x0010-0x1ffd] usable > [0.00] BIOS-e820: [mem 0x1ffe-0x1fff] reserved > [0.00] BIOS-e820: [mem 0xfffc-0x] reserved > [0.00] NX (Execute Disable) protection: active > [0.00] SMBIOS 2.8 present. > [0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > 1.10.2-1ubuntu1 04/01/2014 > [0.00] e820: last_pfn = 0x1ffe0 max_arch_pfn = 0x4 > [0.00] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WP UC- WT > [0.00] found SMP MP-table at [mem 0x000f6aa0-0x000f6aaf] mapped at [ > (ptrval)] > [0.00] Scanning 1 areas for low memory corruption > [0.00] RAMDISK: [mem 0x1fa5f000-0x1ffd] > [0.00] ACPI: Early table checksum verification disabled > [0.00] ACPI: RSDP 0x000F68C0 14 (v00 BOCHS ) > [0.00] ACPI: RSDT 0x1FFE15FC 30 (v01 BOCHS BXPCRSDT > 0001 BXPC 0001) > [0.00] ACPI: FACP 0x1FFE1458 74 (v01 BOCHS BXPCFACP > 0001 BXPC 0001) > [0.00] ACPI: DSDT 0x1FFE0040 001418 (v01 BOCHS BXPCDSDT > 0001 BXPC 0001) > [0.00] ACPI: FACS 0x1FFE 40 > [0.00] ACPI: APIC 0x1FFE154C 78 (v01 BOCHS BXPCAPIC > 0001 BXPC 0001) > [0.00] ACPI: HPET 0x1FFE15C4 38 (v01 BOCHS BXPCHPET > 0001 BXPC 0001) > [0.00] No NUMA configuration found > [0.00] Faking a node at [mem 0x-0x1ffd] > [0.00] NODE_DATA(0) allocated [mem 0x1fa5b000-0x1fa5efff] > [0.00] tsc: Fast TSC calibration using PIT > [0.00] Zone ranges: > [0.00] DMA [mem 0x1000-0x00ff] > [0.00] DMA32[mem 0x0100-0x1ffd] > [0.00] Normal empty >
Re: [PATCH] hfs: fix array out of bounds read of array extent
On Fri, Aug 31, 2018 at 03:05:38PM +0100, Colin King wrote:
> From: Colin Ian King
>
> Currently extent and index i are both being incremented causing
> an array out of bounds read on extent[i]. Fix this by removing
> the extraneous increment of extent.
>
> Detected by CoverityScan, CID#711541 ("Out of bounds read")
>
> Fixes: d1081202f1d0 ("HFS rewrite")
> Signed-off-by: Colin Ian King
I don't think this got picked up yet; let's see if I can help.
Reviewed-by: Ernesto A. Fernández
> ---
> fs/hfs/extent.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
> index 5d0182654580..636cdfcecb26 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
> @@ -300,7 +300,7 @@ int hfs_free_fork(struct super_block *sb, struct
> hfs_cat_file *file, int type)
> return 0;
>
> blocks = 0;
> - for (i = 0; i < 3; extent++, i++)
> + for (i = 0; i < 3; i++)
> blocks += be16_to_cpu(extent[i].count);
>
> res = hfs_free_extents(sb, extent, blocks, blocks);
> --
> 2.17.1
>
Re: [PATCH] hfs: fix array out of bounds read of array extent
On Wed, Oct 17, 2018 at 03:01:17PM -0700, Andrew Morton wrote:
> On Fri, 31 Aug 2018 15:05:38 +0100 Colin King
> wrote:
>
> > From: Colin Ian King
> >
> > Currently extent and index i are both being incremented causing
> > an array out of bounds read on extent[i]. Fix this by removing
> > the extraneous increment of extent.
> >
> > Detected by CoverityScan, CID#711541 ("Out of bounds read")
> >
> > Fixes: d1081202f1d0 ("HFS rewrite")
>
> No such commit here. I assume this is 7cb74be6fd827e314f8.
Sorry, I missed that. This bug has actually been here since before the
first git commit.
>
> > --- a/fs/hfs/extent.c
> > +++ b/fs/hfs/extent.c
> > @@ -300,7 +300,7 @@ int hfs_free_fork(struct super_block *sb, struct
> > hfs_cat_file *file, int type)
> > return 0;
> >
> > blocks = 0;
> > - for (i = 0; i < 3; extent++, i++)
> > + for (i = 0; i < 3; i++)
> > blocks += be16_to_cpu(extent[i].count);
> >
> > res = hfs_free_extents(sb, extent, blocks, blocks);
>
> Well, that's quite the bug. Question is, why didn't anyone notice it.
> What are the runtime effects?
This is only triggered when deleting a file with a resource fork. I may
be wrong because the documentation isn't clear, but I don't think you can
create those under linux. So I guess nobody was testing them.
> A disk space leak, perhaps?
That's what it looks like in general. hfs_free_extents() won't do anything
if the block count doesn't add up, and the error will be ignored. Now, if
the block count randomly does add up, we could see some corruption.
> I worry a bit that, given the fs was evidently working "ok", perhaps
> this error was corrected elsewhere in the code and that "fixing" this
> site will have unexpected and undesirable runtime effects. Can someone
> help me out here?
I don't think so. This bug also makes extent point to the wrong place on
the following call to hfs_free_extents(). There is no way this can work
correctly in general.
Re: [PATCH] hfsplus: stop workqueue when fill_super() failed
On Tue, May 15, 2018 at 07:11:06PM +0900, Tetsuo Handa wrote: > From ffd64dcf946502e7bb1d23c021ee9a4fc92f9312 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa > Date: Tue, 15 May 2018 12:23:03 +0900 > Subject: [PATCH] hfsplus: stop workqueue when fill_super() failed > > syzbot is reporting ODEBUG messages at hfsplus_fill_super() [1]. > This is because hfsplus_fill_super() forgot to call > cancel_delayed_work_sync(). > > As far as I can see, it is hfsplus_mark_mdb_dirty() from > hfsplus_new_inode() in hfsplus_fill_super() that calls > queue_delayed_work(). Therefore, I assume that hfsplus_new_inode() does not > fail if queue_delayed_work() was called, and the out_put_hidden_dir label > is the appropriate location to call cancel_delayed_work_sync(). > > [1] > https://syzkaller.appspot.com/bug?id=a66f45e96fdbeb76b796bf46eb25ea878c42a6c9 > > Signed-off-by: Tetsuo Handa > Reported-by: syzbot > Cc: Al Viro > --- > fs/hfsplus/super.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c > index 9e690ae..80abba5 100644 > --- a/fs/hfsplus/super.c > +++ b/fs/hfsplus/super.c > @@ -590,6 +590,7 @@ static int hfsplus_fill_super(struct super_block *sb, > void *data, int silent) > return 0; > > out_put_hidden_dir: > + cancel_delayed_work_sync(&sbi->sync_work); > iput(sbi->hidden_dir); > out_put_root: > dput(sb->s_root); > -- > 1.8.3.1 > > I sent this same patch a couple of weeks ago: https://www.spinics.net/lists/linux-fsdevel/msg125240.html I should probably have sent it in reply to syzbot to prevent this kind of duplication of work. Sorry about that.
Re: [Linux-kernel-mentees] [PATCH] hfs, hfsplus: Fix NULL pointer dereference in hfs_find_init()
Hi, On Wed, Aug 12, 2020 at 11:59:04AM +0300, Dan Carpenter wrote: > Yeah, the patch doesn't work at all. I looked at one call tree and it > is: > > hfs_mdb_get() tries to allocate HFS_SB(sb)->ext_tree. > > HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp); > > > hfs_btree_open() calls page = read_mapping_page(mapping, 0, NULL); > read_mapping_page() calls mapping->a_ops->readpage() which leads to > hfs_readpage() which leads to hfs_ext_read_extent() which calls > res = hfs_find_init(HFS_SB(inode->i_sb)->ext_tree, &fd); > > > So we need ->ext_tree to be non-NULL before we can set ->ext_tree to be > non-NULL... :/ For HFS+, the first 8 extents for a file are kept inside its own fork data structure, not in the extent tree. So, in normal operation, you don't need to search the extent tree to find the first page of the extent tree itself. The HFS layout is different, but it should work the same way. Of course this sort of thing can still be triggered by crafted filesystems. If that's what the reproducer is about, I think just returning an error is reasonable. But these modules will never be safe against attacks such as this. > I wonder how long this has been broken and if we should just delete the > AFS file system. > > regards, > dan carpenter
Re: [Linux-kernel-mentees] [PATCH] hfs, hfsplus: Fix NULL pointer dereference in hfs_find_init()
On Wed, Aug 12, 2020 at 05:24:20PM -0300, Ernesto A. Fernández wrote: > If that's what the reproducer is about, I think just returning an error is > reasonable. I guess it would be better to put a check inside hfsplus_inode_read_fork(), to verify that the first extent is always in the right place and wide enough.
Re: [PATCH 2/2] hfsplus: add a check for hfs_bnode_find
Hi, On Wed, Oct 16, 2019 at 08:06:20PM +0800, Chuhong Yuan wrote: > hfs_brec_update_parent misses a check for hfs_bnode_find and may miss > the failure. > Add a check for it like what is done in again. > > Signed-off-by: Chuhong Yuan > --- > fs/hfsplus/brec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c > index 1918544a7871..22bada8288c4 100644 > --- a/fs/hfsplus/brec.c > +++ b/fs/hfsplus/brec.c > @@ -434,6 +434,8 @@ static int hfs_brec_update_parent(struct hfs_find_data > *fd) > new_node->parent = tree->root; > } > fd->bnode = hfs_bnode_find(tree, new_node->parent); > + if (IS_ERR(fd->bnode)) > + return PTR_ERR(fd->bnode); You shouldn't just return here, you still hold a reference to new_node. The call to hfs_bnode_find() after the again label seems to be making a similar mistake. I don't think either one can actually fail though, because the parent nodes have all been read and hashed before, haven't they? > /* create index key and entry */ > hfs_bnode_read_key(new_node, fd->search_key, 14); > cnid = cpu_to_be32(new_node->this); > -- > 2.20.1 >
Re: [PATCH 2/2] hfsplus: add a check for hfs_bnode_find
On Thu, Oct 17, 2019 at 09:30:20AM +0800, Chuhong Yuan wrote: > On Thu, Oct 17, 2019 at 8:07 AM Ernesto A. Fernández > wrote: > > > > Hi, > > > > On Wed, Oct 16, 2019 at 08:06:20PM +0800, Chuhong Yuan wrote: > > > hfs_brec_update_parent misses a check for hfs_bnode_find and may miss > > > the failure. > > > Add a check for it like what is done in again. > > > > > > Signed-off-by: Chuhong Yuan > > > --- > > > fs/hfsplus/brec.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/fs/hfsplus/brec.c b/fs/hfsplus/brec.c > > > index 1918544a7871..22bada8288c4 100644 > > > --- a/fs/hfsplus/brec.c > > > +++ b/fs/hfsplus/brec.c > > > @@ -434,6 +434,8 @@ static int hfs_brec_update_parent(struct > > > hfs_find_data *fd) > > > new_node->parent = tree->root; > > > } > > > fd->bnode = hfs_bnode_find(tree, new_node->parent); > > > + if (IS_ERR(fd->bnode)) > > > + return PTR_ERR(fd->bnode); > > > > You shouldn't just return here, you still hold a reference to new_node. > > The call to hfs_bnode_find() after the again label seems to be making a > > similar mistake. > > > > I don't think either one can actually fail though, because the parent > > nodes have all been read and hashed before, haven't they? > > > > I find that after hfs_bnode_findhash in hfs_bnode_find, there is a test for > HFS_BNODE_ERROR and may return an error. I'm not sure whether it > can happen here. That would require a race between hfs_bnode_find() and hfs_bnode_create(), but the node has already been created. > > > > /* create index key and entry */ > > > hfs_bnode_read_key(new_node, fd->search_key, 14); > > > cnid = cpu_to_be32(new_node->this); > > > -- > > > 2.20.1 > > >
[ANNOUNCE] Read-only APFS module
Hi: We've been working on a read-only module for the Apple File System. It's reasonably full-featured (no compression or encryption yet) but not very well tested, so I'm hoping to find some interested users. Questions and criticism are welcome. Git tree: https://github.com/eafer/linux-apfs.git Mailing list: linux-a...@googlegroups.com Patchwork: https://patchwork.kernel.org/project/linux-apfs/list
