Re[2]: kernel oops - do_ip_vs_get_ctl

Re[2]: kernel oops - do_ip_vs_get_ctl

[Hans Schillstrom]

Hello Julian
>
>Hello,
>
>On Fri, 20 Apr 2012, Ryan O'Hara wrote:
>
>> 
>> I frequently get a kernel oops in do_ip_vs_get_ctl when starting keepalived on
>> a 3.3.0 kernel. I'm attaching the trace from /var/log/messages. Has anyone
>> encountered this problem and if so is there a patch available? Much
>> appreciated.
>
>	Is it happening while ip_vs module is loading?
>nf_register_sockopt is one of the first thing that is
>initialized, may be the GET operation accesses something
>from net->ipvs that is not initialized yet. As I can not
>see properly the exact place from the oops, I'll try to
>analyze do_ip_vs_get_ctl for such problem. May be other
>folks will be faster in tracking the right command and
>position in the source code.
>

It's a NULL ptr in  ipvs->num_services; at line 2649

2644            case IP_VS_SO_GET_INFO:
2645            {
2646                    struct ip_vs_getinfo info;
2647                    info.version = IP_VS_VERSION_CODE;
   0x00000000000086a9 <+1481>:  movl   $0x10201,-0x180(%rbp)

2648                    info.size = ip_vs_conn_tab_size;
   0x00000000000086a3 <+1475>:  mov    0x0(%rip),%eax        # 0x86a9 <do_ip_vs_get_ctl+1481>
   0x00000000000086b3 <+1491>:  mov    %eax,-0x17c(%rbp)

2649                    info.num_services = ipvs->num_services;
   0x00000000000086b9 <+1497>:  mov    0x5f8(%r12),%eax
   0x00000000000086c1 <+1505>:  mov    %eax,-0x178(%rbp)
   0x00000000000086c7 <+1511>:  jmpq   0x835d <do_ip_vs_get_ctl+637>

2650                    if (copy_to_user(user, &info, sizeof(info)) != 0)
2651                            ret = -EFAULT;
2652            }
2653            break;
2654
2655            case IP_VS_SO_GET_SERVICES:


>	May be we have to split ip_vs_control_init to
>two parts so that sockopts are the last thing to register
>on init.

I think that is a good idea i.e. add it after nf_register_hooks()

Do you prepare a patch or should I do it  ?

>
>Regards
>
>--
>Julian Anastasov <ja@ssi.bg>
>--

Regards 
Hans Schillstrom
+46 70 699 7150

Re[2]: kernel oops - do_ip_vs_get_ctl

[Julian Anastasov]

	Hello,

On Fri, 20 Apr 2012, Hans Schillstrom wrote:

> >On Fri, 20 Apr 2012, Ryan O'Hara wrote:
> >
> >> 
> >> I frequently get a kernel oops in do_ip_vs_get_ctl when starting keepalived on
> >> a 3.3.0 kernel. I'm attaching the trace from /var/log/messages. Has anyone
> >> encountered this problem and if so is there a patch available? Much
> >> appreciated.
> >
> >	Is it happening while ip_vs module is loading?
> >nf_register_sockopt is one of the first thing that is
> >initialized, may be the GET operation accesses something
> >from net->ipvs that is not initialized yet. As I can not
> >see properly the exact place from the oops, I'll try to
> >analyze do_ip_vs_get_ctl for such problem. May be other
> >folks will be faster in tracking the right command and
> >position in the source code.
> >
> 
> It's a NULL ptr in  ipvs->num_services; at line 2649

	So, it seems ipvs is NULL...

> 2644            case IP_VS_SO_GET_INFO:
> 2645            {
> 2646                    struct ip_vs_getinfo info;
> 2647                    info.version = IP_VS_VERSION_CODE;
>    0x00000000000086a9 <+1481>:  movl   $0x10201,-0x180(%rbp)
> 
> 2648                    info.size = ip_vs_conn_tab_size;
>    0x00000000000086a3 <+1475>:  mov    0x0(%rip),%eax        # 0x86a9 <do_ip_vs_get_ctl+1481>
>    0x00000000000086b3 <+1491>:  mov    %eax,-0x17c(%rbp)
> 
> 2649                    info.num_services = ipvs->num_services;
>    0x00000000000086b9 <+1497>:  mov    0x5f8(%r12),%eax
>    0x00000000000086c1 <+1505>:  mov    %eax,-0x178(%rbp)
>    0x00000000000086c7 <+1511>:  jmpq   0x835d <do_ip_vs_get_ctl+637>
> 
> 2650                    if (copy_to_user(user, &info, sizeof(info)) != 0)
> 2651                            ret = -EFAULT;
> 2652            }
> 2653            break;
> 2654
> 2655            case IP_VS_SO_GET_SERVICES:
> 
> 
> >	May be we have to split ip_vs_control_init to
> >two parts so that sockopts are the last thing to register
> >on init.
> 
> I think that is a good idea i.e. add it after nf_register_hooks()

	Exactly, we can put nf_register_sockopt and
ip_vs_genl_register in some new function to call.

> Do you prepare a patch or should I do it  ?

	I'm stopping doing more patches until Simon takes
the previous changes, so that we can use some fresh tree.
You can try fixing this problem if you think you have
recent changes.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re[2]: kernel oops - do_ip_vs_get_ctl

[Hans Schillstrom]

Hi
>
>Hello,
>
>On Mon, 23 Apr 2012, Simon Horman wrote:
>
>> Hi,
>> 
>> sorry for not being a little more attentive to patches.
>> 
>> I have now picked up all the patches that seem to have consensus.
>> Those that seem critical I have pushed into ipvs with a CC: stable@
>> and sent a pull request to Pablo to consider them for 3.4.
>> 
>> There are two such patches and the head of the ipvs tree now looks like
>> this:
>> 
>> 0cc4789 ipvs: fix crash in ip_vs_control_net_cleanup on unload
>> bd7dc1c netfilter: ipvs: Verify that IP_VS protocol has been registered
>> 
>> Those that seemed less critical where the GFP_ATOMIC changes, one
>> from Sasha and 6 from Julian. The head of the ipvs-next tree now looks like
>> this:
>> 
>> 663f4b2 netfilter: ipvs: use GFP_KERNEL allocation where possible
>> b5cfd04 ipvs: SH scheduler does not need GFP_ATOMIC allocation
>> 5b3b290 ipvs: LBLCR scheduler does not need GFP_ATOMIC allocation on init
>> c087c6f ipvs: WRR scheduler does not need GFP_ATOMIC allocation
>> 8cfaf8d ipvs: DH scheduler does not need GFP_ATOMIC allocation
>> e7c6390 ipvs: LBLC scheduler does not need GFP_ATOMIC allocation on init
>> 8f78609 ipvs: timeout tables do not need GFP_ATOMIC allocation
>> 
>> Please let me know if there are any other patches you would like
>> merged at this time.

This one should also be included
"IPVS: take care of return value from protocol init_netns"
Do you want me to resend it with the others ?

>
>	These two patches are also fixes but may be the 2nd
>patch is too long for fix:
>
>ipvs: reset ipvs pointer in netns
>ipvs: fix app registration in netns
>
>	If it looks too long for fix we can push some
>simple check for net->ipvs in __ip_vs_ftp_init, so that
>we do not oops when IPVS core is compiled in kernel.
>Even if smaller version is sent to stable kernels,
>I prefer "ipvs: fix app registration in netns" to be
>applied at least for net-next. May be I should split
>this 2nd patch as two-line fix + additional change
>for net-next?
>
>	Hans should provide similar two-line fixes for
>LBLC and LBLCR. And one for latest crash report.

I'll send them later today

The IPv6 fragment handling is also ready, but is on hold until above is "stable"

Regards
Hans Schillstrom <hans@schillstrom.com>