On 07/25/2011 12:41 PM, Michal Hocko wrote:
> 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).
>
I talked about this issue with colleagues, you are right - i should fix it.
------------------------------------------------------------------------------
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
