syzbot wrote on Sun, Aug 26, 2018: > HEAD commit: e27bc174c9c6 Add linux-next specific files for 20180824 > git tree: linux-next > console output: https://syzkaller.appspot.com/x/log.txt?x=15dc19a6400000 > kernel config: https://syzkaller.appspot.com/x/.config?x=28446088176757ea > dashboard link: https://syzkaller.appspot.com/bug?extid=d4252148d198410b864f > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15f8efba400000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1178256a400000 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+d4252148d198410b8...@syzkaller.appspotmail.com > > random: sshd: uninitialized urandom read (32 bytes read) > random: sshd: uninitialized urandom read (32 bytes read) > random: sshd: uninitialized urandom read (32 bytes read) > random: sshd: uninitialized urandom read (32 bytes read) > ================================================================== > BUG: KASAN: double-free or invalid-free in p9stat_free+0x35/0x100 > net/9p/protocol.c:48
That looks straight-forward enough, p9pdu_vreadf does p9stat_free on error then v9fs_dir_readdir does the same ; there is nothing else that could return an error without going through the first free so we could just remove the later one... There are a couple other users of the 'S' pdu read (that reads the stat struct and frees it on error), so it's probably best to keep the current behaviour as far as this is concerned, what we could do though is make the free function idempotent (write NULLs in the freed fields), but I do not see this being done often, do you know what the policy is about this kind of pattern nowadays? The struct is cleanly zeroed before being read so there is no risk of double-frees between iterations so zeroing pointers is not strictly required, but it does make things safer in general. -- Dominique Martinet
