r21549: Only create DISP_INFO structs for domain handles, the others don't need

them. It just does not make sense to do a querydispinfo on an alias handle...

This fixes a memleak: Every samr_connect*() call leaked a DISP_INFO for the
(NULL) sid.

More cleanup pending: Essentially, we only need the DISP_INFO cache for the
get_global_sam_sid() domain. BUILTIN is fixed and small enough, and there are
no other domains around where enumerations could happen.

This also removes the explicit builtin_domain flags. I don't think this is
worth it. If this makes a significant difference, then we have a *VERY* tuned
RPC layer...

Jeremy, please check this. If it's ok, we might want to merge it across.

Volker

diff --git a/source/rpc_server/srv_samr_nt.c b/source/rpc_server/srv_samr_nt.c
index d35d97f2a075a5d7ac74a28871311029404530cf..f59ab61509bd9604e6b5531ac8d1a1ff6c6a45e3 100644 (file)
--- a/source/rpc_server/srv_samr_nt.c
+++ b/source/rpc_server/srv_samr_nt.c
@@ -49,7 +49,6 @@ typedef struct disp_info {
        struct disp_info *next, *prev;
        TALLOC_CTX *mem_ctx;
        DOM_SID sid; /* identify which domain this is. */
-       BOOL builtin_domain; /* Quick flag to check if this is the builtin domain. */
        struct pdb_search *users; /* querydispinfo 1 and 4 */
        struct pdb_search *machines; /* querydispinfo 2 */
        struct pdb_search *groups; /* querydispinfo 3 and 5, enumgroups */
@@ -70,7 +69,6 @@ static DISP_INFO *disp_info_list;
 struct samr_info {
        /* for use by the \PIPE\samr policy */
        DOM_SID sid;
-       BOOL builtin_domain; /* Quick flag to check if this is the builtin domain. */
        uint32 status; /* some sort of flag.  best to record it.  comes from opnum 0x39 */
        uint32 acc_granted;
        DISP_INFO *disp_info;
@@ -254,22 +252,11 @@ static NTSTATUS access_check_samr_function(uint32 acc_granted, uint32 acc_requir
  Fetch or create a dispinfo struct.
 ********************************************************************/
 
-static DISP_INFO *get_samr_dispinfo_by_sid(DOM_SID *psid, const char *sid_str)
+static DISP_INFO *get_samr_dispinfo_by_sid(DOM_SID *psid)
 {
        TALLOC_CTX *mem_ctx;
        DISP_INFO *dpi;
 
-       /* There are two cases to consider here:
-          1) The SID is a domain SID and we look for an equality match, or
-          2) This is an account SID and so we return the DISP_INFO* for our 
-             domain */
-
-       if ( psid && sid_check_is_in_our_domain( psid ) ) {
-               DEBUG(10,("get_samr_dispinfo_by_sid: Replacing %s with our domain SID\n",
-                       sid_str));
-               psid = get_global_sam_sid();
-       }
-
        for (dpi = disp_info_list; dpi; dpi = dpi->next) {
                if (sid_equal(psid, &dpi->sid)) {
                        return dpi;
@@ -280,19 +267,18 @@ static DISP_INFO *get_samr_dispinfo_by_sid(DOM_SID *psid, const char *sid_str)
           can get a list out of smbd using smbcontrol. There will
           be one of these per SID we're authorative for. JRA. */
 
-       mem_ctx = talloc_init("DISP_INFO for domain sid %s", sid_str);
+       mem_ctx = talloc_init("DISP_INFO for domain sid %s",
+                             sid_string_static(psid));
 
-       if ((dpi = TALLOC_ZERO_P(mem_ctx, DISP_INFO)) == NULL)
+       if ((dpi = TALLOC_ZERO_P(mem_ctx, DISP_INFO)) == NULL) {
+               DEBUG(0, ("talloc failed\n"));
+               TALLOC_FREE(mem_ctx);
                return NULL;
+       }
 
        dpi->mem_ctx = mem_ctx;
 
-       if (psid) {
-               sid_copy( &dpi->sid, psid);
-               dpi->builtin_domain = sid_check_is_builtin(psid);
-       } else {
-               dpi->builtin_domain = False;
-       }
+       sid_copy( &dpi->sid, psid);
 
        DLIST_ADD(disp_info_list, dpi);
 
@@ -323,20 +309,11 @@ static struct samr_info *get_samr_info_by_sid(DOM_SID *psid)
        DEBUG(10,("get_samr_info_by_sid: created new info for sid %s\n", sid_str));
        if (psid) {
                sid_copy( &info->sid, psid);
-               info->builtin_domain = sid_check_is_builtin(psid);
        } else {
                DEBUG(10,("get_samr_info_by_sid: created new info for NULL sid.\n"));
-               info->builtin_domain = False;
        }
        info->mem_ctx = mem_ctx;
 
-       info->disp_info = get_samr_dispinfo_by_sid(psid, sid_str);
-
-       if (!info->disp_info) {
-               talloc_destroy(mem_ctx);
-               return NULL;
-       }
-
        return info;
 }
 
@@ -493,7 +470,7 @@ static uint32 count_sam_users(struct disp_info *info, uint32 acct_flags)
 {
        struct samr_displayentry *entry;
 
-       if (info->builtin_domain) {
+       if (sid_check_is_builtin(&info->sid)) {
                /* No users in builtin. */
                return 0;
        }
@@ -517,7 +494,7 @@ static uint32 count_sam_groups(struct disp_info *info)
 {
        struct samr_displayentry *entry;
 
-       if (info->builtin_domain) {
+       if (sid_check_is_builtin(&info->sid)) {
                /* No groups in builtin. */
                return 0;
        }
@@ -625,6 +602,11 @@ NTSTATUS _samr_open_domain(pipes_struct *p, SAMR_Q_OPEN_DOMAIN *q_u, SAMR_R_OPEN
                return NT_STATUS_NO_MEMORY;
        info->acc_granted = acc_granted;
 
+       if (!(info->disp_info = get_samr_dispinfo_by_sid(&q_u->dom_sid.sid))) {
+               TALLOC_FREE(info->mem_ctx);
+               return NT_STATUS_NO_MEMORY;
+       }
+
        /* get a (unique) handle.  open a policy on it. */
        if (!create_policy_hnd(p, &r_u->domain_pol, free_samr_info, (void *)info))
                return NT_STATUS_OBJECT_NAME_NOT_FOUND;
@@ -679,6 +661,11 @@ static BOOL get_lsa_policy_samr_sid( pipes_struct *p, POLICY_HND *pol,
        if (!info)
                return False;
 
+       if (!info->disp_info) {
+               /* Not a domain */
+               return False;
+       }
+
        *sid = info->sid;
        *acc_granted = info->acc_granted;
        if (ppdisp_info) {
@@ -911,6 +898,11 @@ NTSTATUS _samr_enum_dom_users(pipes_struct *p, SAMR_Q_ENUM_DOM_USERS *q_u,
        if (!find_policy_by_hnd(p, &q_u->pol, (void **)(void *)&info))
                return NT_STATUS_INVALID_HANDLE;
 
+       if (!info->disp_info) {
+               /* not a domain */
+               return NT_STATUS_INVALID_HANDLE;
+       }
+
        if (!NT_STATUS_IS_OK(r_u->status = access_check_samr_function(info->acc_granted, 
                                        SA_RIGHT_DOMAIN_ENUM_ACCOUNTS, 
                                        "_samr_enum_dom_users"))) {
@@ -919,7 +911,7 @@ NTSTATUS _samr_enum_dom_users(pipes_struct *p, SAMR_Q_ENUM_DOM_USERS *q_u,
        
        DEBUG(5,("_samr_enum_dom_users: %d\n", __LINE__));
 
-       if (info->builtin_domain) {
+       if (sid_check_is_builtin(&info->sid)) {
                /* No users in builtin. */
                init_samr_r_enum_dom_users(r_u, q_u->start_idx, 0);
                DEBUG(5,("_samr_enum_dom_users: No users in BUILTIN\n"));
@@ -1044,6 +1036,11 @@ NTSTATUS _samr_enum_dom_groups(pipes_struct *p, SAMR_Q_ENUM_DOM_GROUPS *q_u, SAM
        if (!find_policy_by_hnd(p, &q_u->pol, (void **)(void *)&info))
                return NT_STATUS_INVALID_HANDLE;
 
+       if (!info->disp_info) {
+               /* not a domain */
+               return NT_STATUS_INVALID_HANDLE;
+       }
+
        r_u->status = access_check_samr_function(info->acc_granted,
                                                 SA_RIGHT_DOMAIN_ENUM_ACCOUNTS,
                                                 "_samr_enum_dom_groups");
@@ -1052,7 +1049,7 @@ NTSTATUS _samr_enum_dom_groups(pipes_struct *p, SAMR_Q_ENUM_DOM_GROUPS *q_u, SAM
 
        DEBUG(5,("samr_reply_enum_dom_groups: %d\n", __LINE__));
 
-       if (info->builtin_domain) {
+       if (sid_check_is_builtin(&info->sid)) {
                /* No groups in builtin. */
                init_samr_r_enum_dom_groups(r_u, q_u->start_idx, 0);
                DEBUG(5,("_samr_enum_dom_users: No groups in BUILTIN\n"));
@@ -1103,6 +1100,11 @@ NTSTATUS _samr_enum_dom_aliases(pipes_struct *p, SAMR_Q_ENUM_DOM_ALIASES *q_u, S
        if (!find_policy_by_hnd(p, &q_u->pol, (void **)(void *)&info))
                return NT_STATUS_INVALID_HANDLE;
 
+       if (!info->disp_info) {
+               /* not a domain */
+               return NT_STATUS_INVALID_HANDLE;
+       }
+
        r_u->status = access_check_samr_function(info->acc_granted,
                                                 SA_RIGHT_DOMAIN_ENUM_ACCOUNTS,
                                                 "_samr_enum_dom_aliases");
@@ -1169,6 +1171,11 @@ NTSTATUS _samr_query_dispinfo(pipes_struct *p, SAMR_Q_QUERY_DISPINFO *q_u,
        if (!find_policy_by_hnd(p, &q_u->domain_pol, (void **)(void *)&info))
                return NT_STATUS_INVALID_HANDLE;
 
+       if (!info->disp_info) {
+               /* not a domain */
+               return NT_STATUS_INVALID_HANDLE;
+       }
+
        /*
         * calculate how many entries we will return.
         * based on 
@@ -2325,6 +2332,11 @@ NTSTATUS _samr_query_domain_info(pipes_struct *p,
        if (!find_policy_by_hnd(p, &q_u->domain_pol, (void **)(void *)&info)) {
                return NT_STATUS_INVALID_HANDLE;
        }
+
+       if (!info->disp_info) {
+               /* not a domain */
+               return NT_STATUS_INVALID_HANDLE;
+       }
        
        switch (q_u->switch_value) {
                case 0x01: