[PATCH] idmap autorid extension to support rid greater than rangesize

[Abhidnya S Joshi]

Hi Christian,

Please find below attached update patch. This takes care of 
1. changing multipler from uint_16 to uint_32 to allow large values.
2. correction in sscanf



Thanks and Regards
Abhidnya Joshi



From:   Christian Ambach <ambi at samba.org>
To:     Abhidnya S Joshi/India/IBM at IBMIN, 
Cc:     samba-technical at samba.org
Date:   04/10/2013 01:18 AM
Subject:        Re: [PATCH] idmap autorid extension to support rid greater 
than rangesize



Hi Abhidnya,

On 04/02/2013 12:38 PM, Abhidnya S Joshi wrote:
> Hi,
>
> Please find attached patch for multiple range support in autorid idmap
> module. This patch applies to master branch.
> The autorid man page says:
> "SIDs with RIDs larger than rangesize cannot be mapped, are ignored and
> the corresponding map is discarded. Choose this value carefully, as this
> should not be changed after the first ranges for domains have been
> defined, otherwise mappings between domains will get intermixed leading 
to
> unpredictable results. As the parameter cannot be changed later, please
> plan accordingly for your expected number of users in a domain with 
safety
> margins."

Would be good if you would also provide an update for the manpage that
removes the limitation as another patch to keep the docs up2date with
the code.

> In production environment, it sometimes becomes difficult to plan max 
rid
> in advance and thus rangesize. With this patch, autorid module will be
> able to support rids larger than rangesize.(Provided range is available)
>
> e.g. The existing method is ID = IDMAP UID LOW VALUE + DOMAINRANGENUMBER 
*
> RANGESIZE + RID
>
> This patch will use ID = IDMAP UID LOW VALUE + DOMAINRANGENUMBER *
> RANGESIZE + RID – ( MULTIPLIER * RANGESIZE)

This can probably also expressed as
ID = LOW VALUE + (DOMAINRANGENUMBER-MULTIPLIER) * RANGESIZE + RID
Correct?

Which formula is used for the opposite direction (xid -> SID)?

> where Multiplier is an unsigned integer  which defines the number of
> ranges to skip while allocating a range. It is calculated by dividing
> incoming RID with RangeSize.
>
> Note: This is just extension to autorid and will not hanmper existing
> functionality

About the patch itself:
+                if (q != NULL)
+                                sscanf(q+1, "%"PRIu16, &multiplier);

This hunk doesn't look correct:
1. you should use SCNu16, PRI is used for printf purposes.
2. you should not ignore the return code of sscanf. A return value other
than 1 indicates an error and then multiplier might have an undefined 
value.

I'll do some functional testing of the patch in the next days, but I am
afraid that one comment that I shared with you earlier is still valid:
There is a potential that this code returns uid/gid values that are
outside of the allowed range.
I'll retest this and let you know about my observations.

Cheers,
Christian




-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-winbindd-autorid-multiple-range-support.patch
Type: application/octet-stream
Size: 5385 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130412/48fc24c6/attachment.obj>