Re: lost connection to test machine (3)

Re: lost connection to test machine (3)

[Dmitry Vyukov]

On Wed, Dec 27, 2017 at 7:18 PM, syzbot
<syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzkaller hit the following crash on
> beacbc68ac3e23821a681adb30b45dc55b17488d
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
> C reproducer is attached
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers
>
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: <syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com>
> It will help syzbot understand when the bug is fixed. See footer for
> details.
> If you forward the report, please keep this part and the footer.

+netfilter maintainers

Here is cleaned reproducer:

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netinet/tcp.h>
#include <linux/if.h>
#include <linux/netfilter_ipv4/ip_tables.h>

int main()
{
  int fd;

  fd = socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
  struct ipt_replace opt = {};
  opt.num_counters = 1;
  opt.size = -1;
  setsockopt(fd, SOL_IP, 0x40, &opt, 0x4);
  return 0;
}


What happens there is that here:

struct xt_table_info *xt_alloc_table_info(unsigned int size)
{
    ...
    if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > totalram_pages)
        return NULL;

size = -1 and SMP_ALIGN(size) = 0, so this still tries to allocate
4GB+delta bytes.

I don't understand why this uses SMP_ALIGN since we add 2 pages on
top, it seems that we could just drop SMP_ALIGN and local SMP_ALIGN
definition altogether.



> ---
> This bug is generated by a dumb bot. It may contain errors.
> See https://goo.gl/tpsmEJ for details.
> Direct all questions to syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report.
> If you forgot to add the Reported-by tag, once the fix for this bug is
> merged
> into any tree, please reply to this email with:
> #syz fix: exact-commit-title
> If you want to test a patch for this bug, please reply with:
> #syz test: git://repo/address.git branch
> and provide the patch inline or as an attachment.
> To mark this as a duplicate of another syzbot report, please reply with:
> #syz dup: exact-subject-of-another-report
> If it's a one-off invalid bug report, please reply with:
> #syz invalid
> Note: if the crash happens again, it will cause creation of a new bug
> report.
> Note: all commands must start from beginning of the line in the email body.
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/001a1143d40c2b55b10561566d26%40google.com.
> For more options, visit https://groups.google.com/d/optout.

Re: lost connection to test machine (3)

[Florian Westphal]

Dmitry Vyukov <dvyukov@google.com> wrote:
> On Wed, Dec 27, 2017 at 7:18 PM, syzbot
> <syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com> wrote:
> > Hello,
> >
> > syzkaller hit the following crash on
> > beacbc68ac3e23821a681adb30b45dc55b17488d
> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> > compiler: gcc (GCC) 7.1.1 20170620
> > .config is attached
> > Raw console output is attached.
> > C reproducer is attached
> > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> > for information about syzkaller reproducers
> >
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: <syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com>
> > It will help syzbot understand when the bug is fixed. See footer for
> > details.
> > If you forward the report, please keep this part and the footer.
> 
> +netfilter maintainers
> 
> Here is cleaned reproducer:
> 
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <netinet/in.h>
> #include <netinet/tcp.h>
> #include <linux/if.h>
> #include <linux/netfilter_ipv4/ip_tables.h>
> 
> int main()
> {
>   int fd;
> 
>   fd = socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
>   struct ipt_replace opt = {};
>   opt.num_counters = 1;
>   opt.size = -1;
>   setsockopt(fd, SOL_IP, 0x40, &opt, 0x4);
>   return 0;
> }
> 
> 
> What happens there is that here:
> 
> struct xt_table_info *xt_alloc_table_info(unsigned int size)
> {
>     ...
>     if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > totalram_pages)
>         return NULL;
> 
> size = -1 and SMP_ALIGN(size) = 0, so this still tries to allocate
> 4GB+delta bytes.
> 
> I don't understand why this uses SMP_ALIGN since we add 2 pages on
> top, it seems that we could just drop SMP_ALIGN and local SMP_ALIGN
> definition altogether.

Looking at history.git this seems to be a left over from back when
iptables allocated size * num_cpus() (and used an SMP_ALIGN based offset
for each cpu).

So yes, I think we can just toss/drop this.

Re: lost connection to test machine (3)

[Dmitry Vyukov]

On Wed, Dec 27, 2017 at 10:36 PM, Florian Westphal <fw@strlen.de> wrote:
> Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Wed, Dec 27, 2017 at 7:18 PM, syzbot
>> <syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com> wrote:
>> > Hello,
>> >
>> > syzkaller hit the following crash on
>> > beacbc68ac3e23821a681adb30b45dc55b17488d
>> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
>> > compiler: gcc (GCC) 7.1.1 20170620
>> > .config is attached
>> > Raw console output is attached.
>> > C reproducer is attached
>> > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>> > for information about syzkaller reproducers
>> >
>> >
>> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> > Reported-by: <syzbot+4396883fa8c4f64e0175@syzkaller.appspotmail.com>
>> > It will help syzbot understand when the bug is fixed. See footer for
>> > details.
>> > If you forward the report, please keep this part and the footer.
>>
>> +netfilter maintainers
>>
>> Here is cleaned reproducer:
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include <sys/types.h>
>> #include <sys/socket.h>
>> #include <netinet/in.h>
>> #include <netinet/tcp.h>
>> #include <linux/if.h>
>> #include <linux/netfilter_ipv4/ip_tables.h>
>>
>> int main()
>> {
>>   int fd;
>>
>>   fd = socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
>>   struct ipt_replace opt = {};
>>   opt.num_counters = 1;
>>   opt.size = -1;
>>   setsockopt(fd, SOL_IP, 0x40, &opt, 0x4);
>>   return 0;
>> }
>>
>>
>> What happens there is that here:
>>
>> struct xt_table_info *xt_alloc_table_info(unsigned int size)
>> {
>>     ...
>>     if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > totalram_pages)
>>         return NULL;
>>
>> size = -1 and SMP_ALIGN(size) = 0, so this still tries to allocate
>> 4GB+delta bytes.
>>
>> I don't understand why this uses SMP_ALIGN since we add 2 pages on
>> top, it seems that we could just drop SMP_ALIGN and local SMP_ALIGN
>> definition altogether.
>
> Looking at history.git this seems to be a left over from back when
> iptables allocated size * num_cpus() (and used an SMP_ALIGN based offset
> for each cpu).
>
> So yes, I think we can just toss/drop this.


Thanks.
I've mailed "netfilter: fix int overflow in xt_alloc_table_info()" to fix this.