Vixie-cron bug

Steve G linux_4ever at yahoo.com
Sat Aug 7 14:28:16 UTC 2004


Hi,
                                                                                
Vixie-cron-4.1 has been recently added to FC. So far, there have been quite
a few complaints about various aspects of it being broken. Cron is a very
important facility and will upset many people if it doesn't work right. With
the freeze is coming in a few weeks, I want to point out a bug in the program.
                                                                                
I usually work with upstream authors on problems like this and don't call
attention to them publicly. I have contacted the author 10 days ago and sent
him the report. He said he will look at it but it seems open-ended as to when
that will happen. I don't have time to fix this and *verify* the solution at the
moment, but I want to detail this one so others who have time might look into 
it.
                                                                                
When I started reviewing the code, I ran across a difference in size
calculations on the dow (day of the week) variable. One place the problem
manifests itself at line 129 of entry.c (there are more places):
                                                                                
           bit_set(e->month, 0);
           bit_nset(e->dow, 0, (LAST_DOW-FIRST_DOW+1));
           e->flags |= DOW_STAR;

For the sake of this discussion, let's take LAST_DOW-FIRST_DOW+1 to be 8.
                                                                                
dow is declared in structs.h as
                                                                                
bitstr_t        bit_decl(dow,    DOW_COUNT);
                                                                                
bit_decl is:
#define bit_decl(name, nbits) \
        (name)[bitstr_size(nbits)]
                                                                                
and
                                                                                
#define bitstr_size(nbits) \
        ((((nbits) - 1) >> 3) + 1)
                                                                                
So, this means that ((8-1)>>3)+1 is 1. dow is an array 1 byte wide -> dow[0].
                                                                                
The bit_nset macro is where the problem comes in. Inside it is this:
                                                                                
register int _stopbyte = _bit_byte(_stop)    <- stop is 8

#define _bit_byte(bit) \
        ((bit) >> 3)
                                                                                
_bit_byte will come up 1 for stopbyte and 0 for startbyte
                                                                                
This means that it will enter the loop in bit_nset:
                                                                                
                _name[_startbyte] |= 0xff << ((_start)&0x7); \
                while (++_startbyte < _stopbyte) \
                        _name[_startbyte] = 0xff; \
                _name[_stopbyte] |= 0xff >> (7 - (_stop&0x7)); \
                                                                                
It will do the startbyte assignment, pre-increment startbyte. Now they are equal
and bypasses the while loop. Then it writes to _name[_stopbyte]...aka dow[1].
dow[0] is legal, but dow[1] is out of range. It will inadvertently write into
an adjacent field of the _entry structure.
                                                                                
So, the question boils down to this: is the dow really 2 bytes or 1 byte? A
simple fix is to change bitstr_size (which is only used for dow):

-       ((((nbits) - 1) >> 3) + 1)
+       (((nbits) >> 3) + 1)
                                                                                
Now, dow is two bytes. No more out of bounds problems. However, I don't think
this is really the best way to solve the problem. One would think 8 bits could
be held in one byte. I haven't researched the impact of changing _bit_byte
since that macro gets used in more places, but more likely to be the
correct fix.
                                                                                
Anyone want to verify this and/or chase it down?
                                                                                
-Steve Grubb


		
__________________________________
Do you Yahoo!?
Take Yahoo! Mail with you! Get it on your mobile phone.
http://mobile.yahoo.com/maildemo 





More information about the fedora-devel-list mailing list