On Mon, Sep 12, 2022 at 02:54:31PM +0800, eadavis@xxxxxxxx wrote: > From: Edward Adam Davis <eadavis@xxxxxxxx> > > the root cause is: > The remaining space after the offset is less than the space needed to > accommodate the next EA_FULL struct. > This needs to be checked on the first iteration as well before calling unpacked_ea_size(ea). if (bytes - *off < sizeof(*ea)) return false; > Link: https://syzkaller.appspot.com/bug?extid=c4d950787fd5553287b7 > Reported-by: syzbot+c4d950787fd5553287b7@xxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Edward Adam Davis <eadavis@xxxxxxxx> > --- > fs/ntfs3/xattr.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c > index 7de8718c68a9..c90cc453390d 100644 > --- a/fs/ntfs3/xattr.c > +++ b/fs/ntfs3/xattr.c > @@ -52,6 +52,7 @@ static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes, > for (;;) { > const struct EA_FULL *ea = Add2Ptr(ea_all, *off); > u32 next_off = *off + unpacked_ea_size(ea); > + u32 next_len = 0; > > if (next_off > bytes) > return false; > @@ -63,6 +64,13 @@ static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes, > *off = next_off; > if (next_off >= bytes) > return false; > + > + next_len = next_off + 8; 8 is a magic number. Use sizeof(*ea). > + if (next_len >= bytes || > + ((!ea->size) && > + (next_len + ea->name_len + > + le16_to_cpu(ea->elength) >= bytes))) This is open coding unpacked_ea_size() but slightly different/incorrect. No need to check this anyway, because it gets checked at the start of the iteration. If we add the if (bytes - *off < sizeof(*ea)) check to the start of the iteration that means we can delete the "if (next_off == bytes)" check from the end. The second issue with this code is that unpacked_ea_size() is a user controlled u32. We're adding it to "*off" which is also a u32 so that can have an integer overflow... See diff below. (Untested). regards, dan carpenter diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c index 7de8718c68a9..c3dbe06fb784 100644 --- a/fs/ntfs3/xattr.c +++ b/fs/ntfs3/xattr.c @@ -44,14 +44,20 @@ static inline size_t packed_ea_size(const struct EA_FULL *ea) static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes, const char *name, u8 name_len, u32 *off) { + const struct EA_FULL *ea; + u32 next_off; + *off = 0; - if (!ea_all || !bytes) + if (!ea_all) return false; for (;;) { - const struct EA_FULL *ea = Add2Ptr(ea_all, *off); - u32 next_off = *off + unpacked_ea_size(ea); + if (bytes - *off < sizeof(*ea)) + return false; + + ea = Add2Ptr(ea_all, *off); + next_off = size_add(*off, unpacked_ea_size(ea)); if (next_off > bytes) return false; @@ -61,8 +67,6 @@ static inline bool find_ea(const struct EA_FULL *ea_all, u32 bytes, return true; *off = next_off; - if (next_off >= bytes) - return false; } }
