于 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:
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
