12.0-BETA1 vnet with pf firewall

[Bjoern A. Zeeb]

On 30 Oct 2018, at 12:23, Kristof Provost wrote:

> On 29 Oct 2018, at 4:41, Kristof Provost wrote:
>> So we panic because we dereference a NULL pointer in strncmp(), which 
>> happens because nprogtab = 13 but ef->progtab[12] has NULL pointers.

I cannot reproduce your panic trying to load both pf, and ipfilter.  I 
could only reproduce the original error.


>> It’s not clear to me why that happens, but it’s something to go 
>> on. I do wonder if this isn’t a bit of a red herring too. It might 
>> be an error in the error path (because we pass through 
>> linker_file_unload()). link_elf_load_file() increments ef->nprogtab 
>> for SHT_X86_64_UNWIND, so perhaps the error handling doesn’t cope 
>> with that.
>>
> It looks like the root of the problem (failure to load) is in this 
> line of link_elf_load_file():
>
> 	ef->progtab[pb].addr =
> 	    vnet_data_alloc(shdr[i].sh_size);
>
> The allocation of the vnet data fails. Bumping VNET_MODMIN in 
> sys/net/vnet.c makes the load of ipfilter and pf succeed.

That’s interesting. I was thinking abut this but the error message (in 
case of not-panic) is only printed in case of ENOSYS:

kern_linker.c:
     486                 /*
     487                  * If the file type has not been recognized by 
the last try
     488                  * printout a message before to fail.
     489                  */
     490                 if (error == ENOSYS)
     491                         printf("%s: %s - unsupported file 
type\n",
     492                             __func__, filename);


And I couldn’t see how we could get to that apart from one case in 
link_obj_elf.c:

     578         if (hdr->e_type != ET_REL) {
     579                 error = ENOSYS;
     580                 goto out;
     581         }

And that’s a very unlikely one.  So the ENOSYS must come out of 
somewhere else.  I haven’t had enough time to track it down.  We 
should.  Could very well be some of the VM calls or something is 
returning that now and we just pass it back?


> I’m not too familiar with this part of the vnet code, but it looks 
> to me like we’ve got more per-vnet variables that was originally 
> anticipated, so we may need to just increase the allocated space.

Can you elfdump -a the two modules and see how big their set_vnet 
section sizes are?  I see:

pf.ko:  sh_size: 6664
ipl.ko: sh_size: 2992

VNET_MODMIN is two pages (8k).  So yes, that would exceed the module 
space.
Having 6.6k global variable space is a bit excessive?  Where does that 
come from?  multicast used to have a similar problem in the past that it 
could not be loaded as a module as it had a massive array there and we 
changed it to be malloced and that reduced it to a pointer.

0000000000000f38 l     O set_vnet       0000000000000428 
vnet_entry_pfr_nulltable
0000000000000b10 l     O set_vnet       00000000000003d0 
vnet_entry_pf_default_rule
0000000000001370 l     O set_vnet       0000000000000690 
vnet_entry_pf_main_anchor
0000000000000000 l     O set_vnet       0000000000000120 
vnet_entry_pf_status

seem to be the biggies.


/bz