On Mon 25-07-11 08:04:41, Nikiforov Alex wrote:
> On 07/22/2011 06:15 PM, Michal Hocko wrote:
> > On Fri 22-07-11 09:06:59, Nikiforov Alex wrote:
> >> @@ -3704,6 +3705,7 @@ int cgroup_register_unchanged_process(pid_t pid,
> >> int flags)
> >> int ret = 1;
> >> char buff[sizeof(CGRULE_SUCCESS_STORE_PID)];
> >> struct sockaddr_un addr;
> >> + struct iovec iov[2] = {};
> >>
> >> sk = socket(PF_UNIX, SOCK_STREAM, 0);
> >> if (sk< 0)
> >> @@ -3720,10 +3722,13 @@ int cgroup_register_unchanged_process(pid_t pid,
> >> int flags)
> >> ret = 0;
> >> goto close;
> >> }
> >> - if (write(sk,&pid, sizeof(pid))< 0)
> >> - goto close;
> >>
> >> - if (write(sk,&flags, sizeof(flags))< 0)
> >> + iov[0].iov_base =&pid;
> >> + iov[0].iov_len = sizeof(pid);
> >> + iov[1].iov_base =&flags;
> >> + iov[1].iov_len = sizeof(flags);
> >> +
> >> + if (writev(sk, iov, 2)< 0)
> >> goto close;
> >
> > shouldn't you check for< iov[0].iov_len + iov[1].iov_len?
> >
> >>
> >> if (read(sk, buff, sizeof(buff))< 0)
> >> diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c
> >> index 2f42a57..14887bb 100644
> >> --- a/src/daemon/cgrulesengd.c
> >> +++ b/src/daemon/cgrulesengd.c
> >> @@ -40,6 +40,7 @@
> >> #include<stdio.h>
> >> #include<stdlib.h>
> >> #include<sys/socket.h>
> >> +#include<sys/uio.h>
> >> #include<sys/syslog.h>
> >> #include<string.h>
> >> #include<linux/netlink.h>
> >> @@ -552,6 +553,7 @@ static void cgre_receive_unix_domain_msg(int sk_unix)
> >> socklen_t caddr_len;
> >> struct stat buff_stat;
> >> char path[FILENAME_MAX];
> >> + struct iovec iov[2] = {};
> >>
> >> caddr_len = sizeof(caddr);
> >> fd_client = accept(sk_unix, (struct sockaddr
> >> *)&caddr,&caddr_len);
> >> @@ -559,16 +561,19 @@ static void cgre_receive_unix_domain_msg(int sk_unix)
> >> cgroup_dbg("accept error: %s\n", strerror(errno));
> >> return;
> >> }
> >> - if (read(fd_client,&pid, sizeof(pid))< 0) {
> >> - cgroup_dbg("read error: %s\n", strerror(errno));
> >> - goto close;
> >> - }
> >> - sprintf(path, "/proc/%d", pid);
> >> - if (stat(path,&buff_stat)) {
> >> - cgroup_dbg("There is not such process (PID: %d)", pid);
> >> - goto close;
> >> - }
> >> - if (read(fd_client,&flags, sizeof(flags))< 0) {
> >> +
> >> + iov[0].iov_base =&pid;
> >> + iov[0].iov_len = sizeof(pid);
> >> + iov[1].iov_base =&flags;
> >> + iov[1].iov_len = sizeof(flags);
> >> +
> >> + if (readv(fd_client, iov, 2)< 0) {
> >
> > Same here.
>
> 1) Dont think that's point here, we are on local iface and we write/read
> just few bytes. For me it's hardly to figure that in this place we can
> get EAGAIN.
The client could send just a pid number without flags you have to be
defensive here as you have no guarantee that the client is behaving...
You would end up with a random garbage from the stack.
> BTW look at previous code, it's twice harder in syscall and
> seems that you dont have any trouble with it
The previous code should have checked the return value properly as well.
It is not that critical, though, because both values could fit also in a
single byte and still have a good meaning. In your case, you have to be
sure that you have read at least one byt for iov[1] (but I think that it
is much better to not be too clever and check the value you are
expecting).
--
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
------------------------------------------------------------------------------
Storage Efficiency Calculator
This modeling tool is based on patent-pending intellectual property that
has been used successfully in hundreds of IBM storage optimization engage-
ments, worldwide. Store less, Store more with what you own, Move data to
the right place. Try It Now! http://www.accelacomm.com/jaw/sfnl/114/51427378/
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
