On 03/06/2013 10:23 PM, KY Srinivasan wrote: > >> -----Original Message----- >> From: vaughan [mailto:vaughan.cao@xxxxxxxxxx] >> Sent: Tuesday, March 05, 2013 9:09 PM >> To: KY Srinivasan >> Cc: Xitao Cao; devel@xxxxxxxxxxxxxxxxxxxxxx; Haiyang Zhang >> Subject: Re: hyperv: is register a mandatory phase by hypervkvpd? >> >> 于 2013年03月06日 01:02, Xitao Cao 写道: >>> On Tue, Mar 5, 2013 at 11:42 PM, KY Srinivasan <kys@xxxxxxxxxxxxx> wrote: >>>> >>>>> -----Original Message----- >>>>> From: vaughan [mailto:vaughan.cao@xxxxxxxxxx] >>>>> Sent: Tuesday, March 05, 2013 7:48 AM >>>>> To: KY Srinivasan >>>>> Cc: devel@xxxxxxxxxxxxxxxxxxxxxx; Haiyang Zhang; xitao.cao@xxxxxxxxx >>>>> Subject: hyperv: is register a mandatory phase by hypervkvpd? >>>>> >>>>> I guess I found a bug -- hypervkvpd running alone without hv_utils >>>>> loaded encounters segfault when service cgred start on RHEL6.4. It >>>>> occurs with both 0.8 and 0.9, regardless of i686 or x86_64. >>>>> >>>>> I read in hv_kvp_daemon.c that the user mode componet should first >>>>> registers with the kernel component. >>>>> But in my test, the hand shake phase has been ignored. >>>>> Things happens like this: >>>>> hv_utils.ko and hv_vmbus.ko is not loaded, start hypervkvpd is fine. >>>>> Then, I start cgred with the default configuration. cgroup also use >>>>> NETLINK_CONNECTOR protocol and send messages with cb_id{1,1}. >> Hypervkvpd >>>>> receive messages without checking their source. Some messages with >>>>> cb_id{1,1} were receviced and blindly interpreted as hv_kvp_msg. Since >>>>> the hand_shake check is as below: > Jason, > There appears to be a connector related issue here. Could you > check to see if the connector code is current. CC Tomas and ShengNan Reproduce this issue, will have a look at it (though not familiar with connector). Maybe Tomas has idea about this. Thanks > Regards, > > K. Y > > There is some connector related issue here. Jason, could you check >>>>> if ((in_hand_shake) && (op == KVP_OP_REGISTER1)) { >>>>> ... >>>>> continue; >>>>> } >>>>> //handle kvp messages >>>>> switch (op) { ... } >>>>> Register phase is also skipped. >>>>> Everytime the KVP_OP_SET opcode is reached, kvp_key_add_or_modify() is >>>>> invoked with an very large key_size. After several iterations, segfault >>>>> occurs in memcpy(record[i].key, key, key_size) (key_size is negative now). >>>>> >>>>> I'm not very familiar with connector. But I ran the sample in >>>>> Documentation/connector/ and found that a NETLINK_CONNECTOR socket >>>>> would >>>>> always some messages with cb_id{1,1}. So blindly suppose all messages >>>>> are kvp_msg is not correct. hypervkvpd should check the source of >>>>> messages and perhaps even check nlmsg_type in the nlmsghdr. >>>>> >>>> The current code does use recvfrom() and checks the sending PID to see if it is >> trusted. >>>> What version of the code are you testing with. >>> Tested both 0.8.0.1 and 0.9.0.1 (redhat version) on 2.6.32-343 on RHEL6.4. >>> I confirm PID is zero, but I think it only means it is sent by kernel. >>> I have no idea whether it is correct for a NETLINK_CONNECTOR >>> implementation that a socket with id {9,1} is able to receive messages >>> with id {1,1}. >> I'm sorry it should be 0-0.9 and 0-0.8 RHEL version. .spec is as below: >> >> # HyperV KVP daemon name >> %global hv_kvp_daemon hv_kvp_daemon >> >> Name: hypervkvpd >> Version: 0 >> Release: 0.9%{?dist} >> Summary: HyperV key value pair (KVP) daemon >> >> Group: System Environment/Daemons >> License: GPLv2 >> URL: http://www.kernel.org >> # Source file obtained from kernel upstream. >> # git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git >> # The daemon and scripts are located in "master branch - /tools/hv" >> Source0: %{hv_kvp_daemon}.c >> Source1: hv_get_dhcp_info.sh >> Source2: hv_get_dns_info.sh >> Source3: hv_set_ifconfig.sh >> Source4: hypervkvpd >> >> # Correct paths to external scripts ("/usr/libexec/hypervkvpd"). >> Patch0: hypervkvpd-0-corrected_paths_to_external_scripts.patch >> # rhbz#872593 >> Patch1: hypervkvpd-0-Netlink-source-address-validation-allows-DoS.patch >> # rhbz#872566 >> Patch2: hypervkvpd-0-long_file_names_from_readdir.patch >> # rhbz#872584 >> Patch3: hypervkvpd-0-var_subdirectory.patch >> Patch4: hypervkvpd-0-string_types.patch >> Patch5: hypervkvpd-0-permissions_of_created_directory_and_files.patch >> # rhbz#890301 >> Patch6: hypervkvpd-0-fix_a_typo_in_hv_set_ifconfig_sh.patch >> Patch7: hypervkvpd-0-Fix-how-ifcfg-file-is-created.patch >> Patch8: hypervkvpd-0-Use-CLOEXEC-when-opening-kvp_pool-files.patch >> >> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) >> # this package is supposed to run on RHEL when it's a guest under Hyper-V >> ExclusiveArch: x86_64 i686 >> BuildRequires: kernel-devel >= 2.6.32-336 >> Requires(post): chkconfig >> Requires(preun): chkconfig >> # This is for /sbin/service >> Requires(preun): initscripts >> Requires(postun): initscripts >> >> %description >> Hypervkvpd is an implementation of HyperV key value pair (KVP) >> functionality for Linux. >> >> >> %prep >> %setup -Tc >> cp -pvL %{SOURCE0} %{hv_kvp_daemon}.c >> cp -pvL %{SOURCE1} hv_get_dhcp_info.sh >> cp -pvL %{SOURCE2} hv_get_dns_info.sh >> cp -pvL %{SOURCE3} hv_set_ifconfig.sh >> cp -pvL %{SOURCE4} hypervkvpd >> >> %patch0 -p1 -b .external_scripts >> %patch1 -p1 -b .netlink_DoS >> %patch2 -p1 -b .long_names >> %patch3 -p1 -b .var_subdir >> %patch4 -p1 -b .string_types >> %patch5 -p1 -b .permissions >> %patch6 -p1 >> %patch7 -p3 -b .ifcfg_fix >> %patch8 -p3 -b .use_CLOEXEC >> >> >> %build >> # kernel-devel version >> %{!?kversion: %global kversion `ls %{_usrsrc}/kernels | sort -dr | head >> -n 1`} >> >> gcc \ >> %{optflags} \ >> -I%{_usrsrc}/kernels/%{kversion}/include \ >> %{hv_kvp_daemon}.c \ >> -o %{hv_kvp_daemon} >> >> >> %install >> rm -rf %{buildroot} >> >> mkdir -p %{buildroot}%{_sbindir} >> install -p -m 0755 %{hv_kvp_daemon} %{buildroot}%{_sbindir} >> mkdir -p %{buildroot}%{_initrddir} >> # SysV init script >> install -p -m 0755 %{SOURCE4} %{buildroot}%{_initrddir} >> # Shell scripts for the daemon >> mkdir -p %{buildroot}%{_libexecdir}/%{name} >> install -p -m 0755 hv_get_dhcp_info.sh >> %{buildroot}%{_libexecdir}/%{name}/hv_get_dhcp_info >> install -p -m 0755 hv_get_dns_info.sh >> %{buildroot}%{_libexecdir}/%{name}/hv_get_dns_info >> install -p -m 0755 hv_set_ifconfig.sh >> %{buildroot}%{_libexecdir}/%{name}/hv_set_ifconfig >> # Directory for pool files >> mkdir -p %{buildroot}%{_sharedstatedir}/hyperv >> >> >> %post >> # This adds the proper /etc/rc*.d links for the script >> /sbin/chkconfig --add hypervkvpd >> >> >> %preun >> # Removing package >> if [ $1 -eq 0 ] ; then >> /sbin/service hypervkvpd stop >/dev/null 2>&1 >> /sbin/chkconfig --del hypervkvpd >> fi >> >> >> %postun >> # Updating package >> if [ "$1" -ge "1" ] ; then >> /sbin/service hypervkvpd condrestart >/dev/null 2>&1 || : >> fi >> # If removing the package, delete %%{_sharedstatedir}/hyperv directory >> if [ "$1" -eq "0" ] ; then >> rm -rf %{_sharedstatedir}/hyperv || : >> fi >> >> >> %clean >> rm -rf %{buildroot} >> >> >> %files >> %defattr(-,root,root,-) >> %doc >> %{_sbindir}/%{hv_kvp_daemon} >> %{_initrddir}/hypervkvpd >> %dir %{_libexecdir}/%{name} >> %{_libexecdir}/%{name}/hv_get_dhcp_info >> %{_libexecdir}/%{name}/hv_get_dns_info >> %{_libexecdir}/%{name}/hv_set_ifconfig >> %dir %{_sharedstatedir}/hyperv >> >> >> %changelog >> * Mon Jan 14 2013 Tomas Hozza <thozza@xxxxxxxxxx> - 0-0.9 >> - Fix a typo in hv_set_ifconfig.sh script >> - Fix creation process of ifcfg-* files when doing IP injection >> - Use CLOEXEC when opening pool files to prevent FD leaking >> - Changes in SPEC file: >> - remove/create %%{_sharedstatedir}/hyperv if removing/installing package >> - install scripts without ".sh" extension >> >> * Mon Nov 12 2012 Tomas Hozza <thozza@xxxxxxxxxx> - 0-0.8 >> - Bumping release to 0.8 to avoid update path from RHEL5 >> - changing the way how Netlink source address is validated (#872593) >> - fix for long names from readdir (#872566) >> - applied reasonable Debian/Ubuntu patches (#872584) >> >> * Thu Nov 08 2012 Tomas Hozza <thozza@xxxxxxxxxx> - 0-0.2 >> - Changing ExclusiveArch just to x86_64 i686 (#870288) >> >> * Tue Sep 25 2012 Tomas Hozza <thozza@xxxxxxxxxx> - 0-0.1 >> - Initial spec file >> >> >>>> Regards, >>>> >>>> K. Y >>>>> -- >>>>> Regards, >>>>> Vaughan >>>>> >>>>> >> _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
