Quota patches
Jeremy Allison
jra at samba.org
Wed Jul 25 19:02:46 UTC 2018
On Tue, Jul 24, 2018 at 10:29:23AM +0100, Noel Power via samba-technical wrote:
> Hi Jeremy
>
> I promised at sambaxp to bug you again with the quota patches for smb2
> (which also include some rewrite of the existing code to use idl). Aside
> from the fact we need to step away from SMB1 we have some customer
> interest in this. I hope you can find some time to have a look. I've
> rebased the patches (attached) against current master
Getting there.
In fill_quota_buffer(), uint32_t max_data is always non-zero,
so I'd prefer this code:
if (max_data && ((header_ndr->offset - next_align)
+ + sizeof(info.next_entry_offset)
+ + sizeof(info.sid_length)
+ + sizeof(info.change_time)
+ + sizeof(info.quota_used)
+ + sizeof(info.quota_threshold)
+ + sizeof(info.quota_limit)
+ + info.sid_length) > max_data) {
+ max_data_exceeded = true;
+ break;
+ }
to look like this:
uint32_t dsize = sizeof(info.next_entry_offset)
+ sizeof(info.sid_length)
+ sizeof(info.change_time)
+ sizeof(info.quota_used)
+ sizeof(info.quota_threshold)
+ sizeof(info.quota_limit)
if (dsize > max_data) {
max_data_exceeded = true;
break;
}
if (dsize + info.sid_length < dsize) {
/* Integer wrap */
max_data_exceeded = true;
break;
}
if (dsize + info.sid_length > max_data) {
max_data_exceeded = true;
break;
}
Also, in:
extract_sids_from_buf()
+ blob.data = sid_buf->data + offset;
Above needs protection that sid_buf->data + offset doesn't
go off the end of sid_buf->length
+ blob.length = sidlistlength - offset;
Likewise doesn't this also need a test that sidlistlength
fits within sid_buf ?
*Anything* that does arithmetic in a ranged buffer blob
needs really careful examination.
This:
+ if (new_offset && (sidlistlength - offset - new_offset) <= 0) {
is too complicated to easily evaluate. Please
break it down to separate statements so I can
understand exactly what you're checking for here.
Use helper variables to make things clearer.
Having said that I *LOVE* the conversion to
IDL-based code. This is a significant improvement
if we can just get the range checking past my
creaking old brain :-).
I know I'm paranoid about this stuff, but it pays
off in the long run :-).
Cheers,
Jeremy.
More information about the samba-technical
mailing list