On Tuesday, January 22, 2013 2:46:47 AM UTC+5:30, Mike Christie wrote:
>
> On 01/21/2013 06:57 AM, rahul wrote: 
> > 
> > 
> > On Monday, January 21, 2013 6:54:33 PM UTC+5:30, manish singh wrote: 
> > 
> >     Hi Rahul, 
> >       
> >     Please see inline 
> > 
> >     On Mon, Jan 21, 2013 at 12:24 PM, rahul <[email protected] 
> >     <javascript:>> wrote: 
> > 
> >         Guys, 
> > 
> >             I see few problems with the function idbm_recinfo_config(). 
> >         Can someone please take a look and confirm ? 
> > 
> >         void idbm_recinfo_config(recinfo_t *info, FILE *f) 
> >         { 
> >         char name[NAME_MAXVAL]; 
> >         char value[VALUE_MAXVAL]; 
> >         char *line, *nl, buffer[2048]; 
> >         int line_number = 0; 
> >         int c = 0, i; 
> > 
> >         fseek(f, 0, SEEK_SET); 
> > 
> >         /* process the config file */ 
> >         do { 
> >         line = fgets(buffer, sizeof (buffer), f); 
> >         line_number++; 
> >         if (!line) 
> >         continue; 
> > 
> >         nl = line + strlen(line) - 1; 
> >         if (*nl != '\n') { 
> >         log_warning("Config file line %d too long.", 
> >               line_number); 
> >         continue; 
> >         } 
> > 
> >         Here, if the line is too long that it cannot fit into the 
> >         buffer, shouldn't we need to ignore the rest of the line ? 
> >           
> > 
> >       
> >     I dont see anything worng here fgets reads either till end of line 
> >     or till given size. 
> >     Refer to link 
> >     http://pubs.opengroup.org/onlinepubs/007904875/functions/fgets.html 
> >     <http://pubs.opengroup.org/onlinepubs/007904875/functions/fgets.html> 
>
> >       
> >     The fgets() function shall read bytes from stream into the array 
> >     pointed to by s, until n-1 bytes are read, or a <newline> is read 
> >     and transferred to s, or an end-of-file condition is encountered. 
> >     The string is then terminated with a null byte. 
> > 
> > 
> > The problem is that if the line is too long (> 2048 chars), then in the 
> > next iteration it will start reading after 2048 characters which doesn't 
> > look correct. If the line is longer than 2048 chars we should skip 
> > anything beyond 2048 on that line. 
>
> Yeah, for really long param=settings strings then it is busted and the 
> code will not read in the value correctly. Continuing to read that line 
> will not help unless we get lucky and all those chars before the 
> param=setting string are whitespaces and we end up just right in the 
> strings and not miss any of the info in the string. 
>
> Right now, I do not think we have any setting that long though, so it 
> does not come up, and having tons of whitespace also does not come up. 
>
> I do not know if any one has time to fix. Is there another bug you are 
> thinking about (if there is a segfault or corrupter or security issue 
> then we should fix) or are you hitting a issue with really long lines in 
> a real setup, or is this one of those things where incorrect code is 
> just irking you and you want it fixed. 
>
> Thanks Mike for taking a look at this. On a real set up we might not hit 
these issues. I was just going through this code when I noticed this.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/open-iscsi/-/JG_2PXIcg24J.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/open-iscsi?hl=en.

Reply via email to