You say: you are still vulnerable - where? to what? On Sun, Jun 23, 2013 at 9:04 AM, Petr Bena <[email protected]> wrote: > Purpose of the struct is that if I forgot to close some FD it would > get automagically closed once the object is destroyed (it's created on > stack, so once it get removed from stack it get auto-closed) > > your change is correct, I should have make sure that closing it > multiple times doesn't cause troubles, however I don't see where I am > closing it three times? > > On Sun, Jun 23, 2013 at 12:25 AM, Platonides <[email protected]> wrote: >> On 22/06/13 20:24, Petr Bena wrote: >>> >>> resolved. >> >> >> No, you're still vulnerable (1609de5f). >> Moreover, your are changing a file different than the one you checked. >> >> I also noticed that you are closing each descriptor thrice. >> >> One instance is trivially fixed by: >>> >>> diff --git a/include/Take.h b/include/Take.h >>> index dd063f3..4fa2a41 100644 >>> --- a/include/Take.h >>> +++ b/include/Take.h >>> @@ -20,7 +20,7 @@ class Take >>> int fd; >>> FD(int f): fd(f) { }; >>> ~FD() { Close(); }; >>> - void Close() { if(fd >= 0) close(fd); }; >>> + void Close() { if(fd >= 0) close(fd); fd = -1; >>> }; >>> operator int (void) const { return fd; }; >>> }; >> >> >> >> The other close was hidden in copied objects (why did you need an object? >> This would have been much easier simply passing the fds!) >> >>> diff --git a/include/Take.h b/include/Take.h >>> index dd063f3..ac8a417 100644 >>> --- a/include/Take.h >>> +++ b/include/Take.h >>> @@ -29,10 +29,10 @@ class Take >>> virtual ~Take(); >>> protected: >>> private: >>> - bool Overtake(string path, FD fd); >>> + bool Overtake(string path, FD& fd); >>> bool Verify(string path); >>> static int Callback(const char* path, const struct stat *sb, int >>> typeflag, struct FTW *ftwbuf); >>> - static void ChangeOwner(string path, uid_t owner, FD fd, bool >>> ChangeGroup); >>> + static void ChangeOwner(string path, uid_t owner, FD& fd, bool >>> ChangeGroup); >>> static bool CheckGroups(struct stat info); >>> static bool CheckHL(string path); >>> static bool CheckHL(struct stat info); >>> diff --git a/src/Take.cpp b/src/Take.cpp >>> index 8a111d6..accfc02 100644 >>> --- a/src/Take.cpp >>> +++ b/src/Take.cpp >>> @@ -126,7 +126,7 @@ int Take::Callback(const char* path, const struct stat >>> *sb, int typeflag, struct >>> return 0; >>> } >>> >>> -void Take::ChangeOwner(string path, uid_t owner, Take::FD fd, bool >>> ChangeGroup) >>> +void Take::ChangeOwner(string path, uid_t owner, Take::FD& fd, bool >>> ChangeGroup) >>> { >>> if (!ChangeGroup) >>> { >>> @@ -215,7 +215,7 @@ bool Take::CheckHL(string path) >>> return false; >>> } >>> >>> -bool Take::Overtake(string path, Take::FD fd) >>> +bool Take::Overtake(string path, Take::FD& fd) >>> { >>> char * p = realpath(path.c_str(), NULL); >>> >> >> >> _______________________________________________ >> Labs-l mailing list >> [email protected] >> https://lists.wikimedia.org/mailman/listinfo/labs-l
_______________________________________________ Labs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/labs-l
