Le 29/12/2012 08:35, Philip Guenther a écrit :
> On Fri, Dec 28, 2012 at 10:16 PM, Maxime Villard <[email protected]> wrote:
>> Le 29/12/2012 02:46, Philip Guenther a écrit :
>>> On Thu, Dec 27, 2012 at 5:04 AM, Maxime Villard <[email protected]> wrote:
>>>> Well, as no one seems to give a fuck on tech@, I put a more
>>>> glamourous title here.
>>>
>>> The fd/FILE part of your diff changes the behavior of pfctl to be
>>> incorrect when there are no states.
>>
>> Hum,
>>
>> Before:
>> - we open the file, go in the loop to do ioctl stuff,
>> and if it fails BAM we lose the fd when returning at l.1893 or
>> l.1889. If it worked, we write data to the file and close it.
>
> And if there are no states at all, such that the first call to
> ioctl(DIOCGETSTATES) returns with ps_len=0, then pfctl_state_store()
> returns without closing the file but *AFTER* it created the file. The
> file is, of course, closed when the process later exits, leaving it a
> zero byte file.
>
>
>> Now:
>> - we go in the loop, we do ioctl stuff, and then if nothing
>> failed we open the file and write data to it, and close it.
>> If something failed in the loop, we return without leaking f.
>
> AND WITHOUT CREATING THE FILE.
>
--- pfctl.c 2012-09-19 17:52:17.000000000 +0200
+++ pfctl.c 2013-01-01 16:07:10.388266823 +0100
@@ -1885,12 +1885,16 @@
if (ps.ps_len + sizeof(struct pfioc_states) < len)
break;
- if (len == 0 && ps.ps_len == 0)
+ if (len == 0 && ps.ps_len == 0) {
+ fclose(f);
return;
+ }
if (len == 0 && ps.ps_len != 0)
len = ps.ps_len;
- if (ps.ps_len == 0)
+ if (ps.ps_len == 0) {
+ fclose(f);
return; /* no states */
+ }
len *= 2;
}
>
>> The fd is not used in the loop, I just moved it down. So I don't
>> see what behaviour it changes.
>
> Uh huh. You didn't try it either.
>
>
> Philip Guenther
