Linux-XFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] xfs: fix mount failure crash on invalid iclog memory access
@ 2019-12-03 14:05 Brian Foster
  2019-12-03 14:09 ` Brian Foster
  2019-12-03 15:52 ` Darrick J. Wong
  0 siblings, 2 replies; 3+ messages in thread
From: Brian Foster @ 2019-12-03 14:05 UTC (permalink / raw)
  To: linux-xfs

syzbot (via KASAN) reports a use-after-free in the error path of
xlog_alloc_log(). Specifically, the iclog freeing loop doesn't
handle the case of a fully initialized ->l_iclog linked list.
Instead, it assumes that the list is partially constructed and NULL
terminated.

This bug manifested because there was no possible error scenario
after iclog list setup when the original code was added.  Subsequent
code and associated error conditions were added some time later,
while the original error handling code was never updated. Fix up the
error loop to terminate either on a NULL iclog or reaching the end
of the list.

Reported-by: syzbot+c732f8644185de340492@syzkaller.appspotmail.com
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_log.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 6a147c63a8a6..f6006d94a581 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1542,6 +1542,8 @@ xlog_alloc_log(
 		prev_iclog = iclog->ic_next;
 		kmem_free(iclog->ic_data);
 		kmem_free(iclog);
+		if (prev_iclog == log->l_iclog)
+			break;
 	}
 out_free_log:
 	kmem_free(log);
-- 
2.20.1


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

* Re: [PATCH] xfs: fix mount failure crash on invalid iclog memory access
  2019-12-03 14:05 [PATCH] xfs: fix mount failure crash on invalid iclog memory access Brian Foster
@ 2019-12-03 14:09 ` Brian Foster
  2019-12-03 15:52 ` Darrick J. Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Brian Foster @ 2019-12-03 14:09 UTC (permalink / raw)
  To: linux-xfs

On Tue, Dec 03, 2019 at 09:05:24AM -0500, Brian Foster wrote:
> syzbot (via KASAN) reports a use-after-free in the error path of
> xlog_alloc_log(). Specifically, the iclog freeing loop doesn't
> handle the case of a fully initialized ->l_iclog linked list.
> Instead, it assumes that the list is partially constructed and NULL
> terminated.
> 
> This bug manifested because there was no possible error scenario
> after iclog list setup when the original code was added.  Subsequent
> code and associated error conditions were added some time later,
> while the original error handling code was never updated. Fix up the
> error loop to terminate either on a NULL iclog or reaching the end
> of the list.
> 
> Reported-by: syzbot+c732f8644185de340492@syzkaller.appspotmail.com
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---

Hmm.. I didn't realize Hillf Danton already replied to the original
thread with this same fix until I looked at the ML archive. His reply
isn't in my mailbox for some reason. Anyways, feel free to skip this
patch in favor of that one..

Brian

>  fs/xfs/xfs_log.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 6a147c63a8a6..f6006d94a581 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1542,6 +1542,8 @@ xlog_alloc_log(
>  		prev_iclog = iclog->ic_next;
>  		kmem_free(iclog->ic_data);
>  		kmem_free(iclog);
> +		if (prev_iclog == log->l_iclog)
> +			break;
>  	}
>  out_free_log:
>  	kmem_free(log);
> -- 
> 2.20.1
> 


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

* Re: [PATCH] xfs: fix mount failure crash on invalid iclog memory access
  2019-12-03 14:05 [PATCH] xfs: fix mount failure crash on invalid iclog memory access Brian Foster
  2019-12-03 14:09 ` Brian Foster
@ 2019-12-03 15:52 ` Darrick J. Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2019-12-03 15:52 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs

On Tue, Dec 03, 2019 at 09:05:24AM -0500, Brian Foster wrote:
> syzbot (via KASAN) reports a use-after-free in the error path of
> xlog_alloc_log(). Specifically, the iclog freeing loop doesn't
> handle the case of a fully initialized ->l_iclog linked list.
> Instead, it assumes that the list is partially constructed and NULL
> terminated.
> 
> This bug manifested because there was no possible error scenario
> after iclog list setup when the original code was added.  Subsequent
> code and associated error conditions were added some time later,
> while the original error handling code was never updated. Fix up the
> error loop to terminate either on a NULL iclog or reaching the end
> of the list.
> 
> Reported-by: syzbot+c732f8644185de340492@syzkaller.appspotmail.com
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Ahh, a proper S-o-B!  Thank you,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_log.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 6a147c63a8a6..f6006d94a581 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1542,6 +1542,8 @@ xlog_alloc_log(
>  		prev_iclog = iclog->ic_next;
>  		kmem_free(iclog->ic_data);
>  		kmem_free(iclog);
> +		if (prev_iclog == log->l_iclog)
> +			break;
>  	}
>  out_free_log:
>  	kmem_free(log);
> -- 
> 2.20.1
> 

^ 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 --
2019-12-03 14:05 [PATCH] xfs: fix mount failure crash on invalid iclog memory access Brian Foster
2019-12-03 14:09 ` Brian Foster
2019-12-03 15:52 ` Darrick J. Wong

Linux-XFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-xfs/0 linux-xfs/git/0.git

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

Example config snippet for mirrors

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


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