| Message ID | 20191203140524.36043-1-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: fix mount failure crash on invalid iclog memory access | expand |
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 >
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 >
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);
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(+)