[PATCH v2] extend sparse file support

Jeremy Allison jra at samba.org
Mon Mar 9 14:32:18 MDT 2015


On Mon, Mar 09, 2015 at 05:31:01PM +0100, David Disseldorp wrote:
> Thanks a lot for the review Jeremy!
> Please see new version attached...
> 
> On Fri, 6 Mar 2015 16:06:45 -0800, Jeremy Allison wrote:
> 
> > +       ndr_ret = ndr_pull_struct_blob(in_input, mem_ctx, &qar_req,
> > +               (ndr_pull_flags_fn_t)ndr_pull_fsctl_query_alloced_ranges_req);
> > 
> > Can you do wrap checks around the len and offset fields in
> > the read structure please. For example:
> > 
> > +       /* maximum offset is either the EOF, or the request off + len */
> > +       max_off = MIN(sbuf.st_ex_size,
> > +                     qar_req.buf.file_off + qar_req.buf.len) - 1;
> > 
> > I'd like wrap checks for 'qar_req.buf.file_off + qar_req.buf.len'
> > and any other arithmetic done on client-supplied values.
> 
> I think the QAR max_off calculation is the only operation susceptible to
> an integer over/underflow, given that we filter out the following
> conditions:
> 359         if ((qar_req.buf.len == 0)
> 360          || (sbuf.st_ex_size == 0)
> 361          || (qar_req.buf.file_off >= sbuf.st_ex_size)) {
> 362                 /* zero length range or after EOF, no ranges to return */
> 363                 return NT_STATUS_OK;
> 364         }
> 
> ...and for ZERO_DATA:
> 158         if (zdata_info.beyond_final_zero < zdata_info.file_off) {
> ...
> 162                 return NT_STATUS_INVALID_PARAMETER;
> 163         }
> 
> I've added a new QAR max_off overflow check, and corresponding torture
> test.
> 
> > Also can you add an explaination for the -1 in the comment here please ?
> 
> Done.

LGTM David and thanks for the ASCII art (I'm a big fan of that :-) !
Pushed.


More information about the samba-technical mailing list