Bug 4395 - 3 Bugs found in reg_backend_rpc.c, reg_util.c and patchfile.c
3 Bugs found in reg_backend_rpc.c, reg_util.c and patchfile.c
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: Other
unspecified
x86 Linux
: P3 normal
: ---
Assigned To: Jelmer Vernooij
Andrew Bartlett
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-14 04:59 UTC by os2a bto
Modified: 2008-01-07 08:24 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description os2a bto 2007-02-14 04:59:31 UTC
Bugs Report
===========

Product : Samba

Version : 4.0.0tp4 

Component : Registry

OS : Red Hat 9

Bug 1 
=====

Description : 
A bug exists in function, get_abs_parent() in reg_util.c which is intended to
get the parent name and registry key name from a given registry key absolute 
path. It computes and returns incorrect value of parent_name from a given 
absolute registry path. This results in addition & deletion opearations of a
registry key to fail via reg_key_del_abs() & reg_key_add_abs() function 
calls.

For Eg. When registry path is "HKEY_LOCAL_MACHINE\Software\Microsoft\test", 
it returns parent_name as "HKEY_LOCAL_MACHINE\Software\Microsof".

Line No. : 199

Affected Code :
parent_name = talloc_strndup(mem_ctx, path, strrchr(path, '\\')-1-path);

Fix : 
Replaced strrchr() to strchr() to return a pointer to the first occurrence of
the character '\\' in the string 'path' and strip only 'path' from this output. 

Patch :
Patch against Samba 4.0.0tp4 is given below. 

# Patch for reg_utilc.c #

--- samba-4.0.0tp4.orig/source/lib/registry/common/reg_util.c   2006-09-28 12:14:47.000000000 +0530
+++ samba-4.0.0tp4/source/lib/registry/common/reg_util.c        2007-02-10 14:31:03.000000000 +0530
@@ -196,7 +196,8 @@
                return WERR_FOOBAR;
        }

-       parent_name = talloc_strndup(mem_ctx, path, strrchr(path, '\\')-1-path);
+       //parent_name = talloc_strndup(mem_ctx, path, strrchr(path, '\\')-1-path);
+       parent_name = talloc_strndup(mem_ctx, path, strchr(path, '\\')-path);

        error = reg_open_key_abs(mem_ctx, ctx, parent_name, parent);
        if (!W_ERROR_IS_OK(error)) {


Bug 2 
=====

Description :
A bug exists in function, rpc_get_value_by_index() in reg_backend_rpc.c which
is intended to get a registry key value from an opened registry key by index.
It has been incorrectly assigned the data type of registry value in this 
function which leads to return imroper data type of the retrieved value for a
given registry key.

Line no. : 207

Affected code : 
(*value)->data_type = type;

Fix :
Modified the member 'data_type' of structure 'value' to refer the actual 
output pointer 'r.out.type'

Patch : 
Patch against Samba 4.0.0tp4 is given below. 

# Patch for reg_backend_rpc.c #

--- samba-4.0.0tp4.orig/source/lib/registry/reg_backend_rpc.c   2006-09-16 00:04:03.000000000 +0530
+++ samba-4.0.0tp4/source/lib/registry/reg_backend_rpc.c        2007-02-10 14:30:17.000000000 +0530
@@ -204,7 +204,7 @@
           W_ERROR_IS_OK(r.out.result) && r.out.length) {
                *value = talloc(mem_ctx, struct registry_value);
                (*value)->name = talloc_strdup(mem_ctx, r.out.name->name);
-               (*value)->data_type = type;
+               (*value)->data_type = *r.out.type;
                (*value)->data = data_blob_talloc(mem_ctx, r.out.value, *r.out.length);
                return WERR_OK;
        }

Bug 3 
=====

Description :
A regpatch utility which is used to apply registry patches for Windows 
Registry, is not able to add a new registry key due to a bug in function
reg_diff_apply() in patchfile.c. Invalid check has been done to add a 
registry key since the registry key is added only when error status from 
reg_open_key_abs() is WERR_DEST_NOT_FOUND. But it is obseverd that 
reg_open_key_abs() does not return WERR_DEST_NOT_FOUND when given registry key
does not exist in the Remote Registry. Hence adding a new registry key was 
never successful via regpatch.

Line no. : 392

Affected code : 
 if (W_ERROR_EQUAL(error, WERR_DEST_NOT_FOUND)) {

Fix :
Modified the code to check iff error status is not WERROR_OK instead of 
checking for the error code WERR_DEST_NOT_FOUND only.

Patch : 
Patch against Samba 4.0.0tp4 is given below. 

# Patch for patchfile.c #

--- samba-4.0.0tp4.orig/source/lib/registry/patchfile.c 2006-03-06 15:39:53.000000000 +0530
+++ samba-4.0.0tp4/source/lib/registry/patchfile.c      2007-02-10 15:37:34.000000000 +0530
@@ -389,7 +389,8 @@
                error = reg_open_key_abs(mem_ctx, ctx, diff->keys[i].name, &tmp);

                /* If we found it, apply the other bits, else create such a key */
-               if (W_ERROR_EQUAL(error, WERR_DEST_NOT_FOUND)) {
+               //if (W_ERROR_EQUAL(error, WERR_DEST_NOT_FOUND)) {
+               if (!W_ERROR_IS_OK(error)) {
                        if(!W_ERROR_IS_OK(reg_key_add_abs(mem_ctx, ctx, diff->keys[i].name, 0, NULL, &tmp))) {
                                DEBUG(0, ("Error adding new key '%s'\n", diff->keys[i].name));
                                return False;
Comment 1 Matthias Dieter Wallnöfer 2007-08-31 05:29:59 UTC
Okay, we should update the bug:
- Bug 1 seems not to be reproducible anymore, because the file doesn't exist in actual SVNs
- Bug 2 the same thing
- Bug 3: the patchfile.c changed so much, that the positions indicated in the patch aren't corrected anymore

Either I would retest the bug or if no one replies I would mark it as closed, because the things are likely to be fixed in the new registry backend ported back to SAMBA 4 a few days ago.
Comment 2 os2a bto 2007-09-18 08:02:12 UTC
(In reply to comment #1)
> Okay, we should update the bug:
> - Bug 1 seems not to be reproducible anymore, because the file doesn't exist in
> actual SVNs
> - Bug 2 the same thing
> - Bug 3: the patchfile.c changed so much, that the positions indicated in the
> patch aren't corrected anymore
> Either I would retest the bug or if no one replies I would mark it as closed,
> because the things are likely to be fixed in the new registry backend ported
> back to SAMBA 4 a few days ago.


This was reported against Samba TP4 build and not on the regular Samba 4 release build. We still see the issue in Samba TP5 and all the relevant files exist and bugs are reproduceable.
Comment 3 Matthias Dieter Wallnöfer 2007-09-20 07:01:20 UTC
Please try to reproduce it with a *newer* release of SAMBA 4 (Alpha 1 or a recent SVN)!
Comment 4 Matthias Dieter Wallnöfer 2008-01-01 14:19:09 UTC
I think this is worth to be closed, because the old registry backend doesn't exist anymore in the SAMBA Alpha releases.
Comment 5 Andrew Bartlett 2008-01-07 00:11:02 UTC
Can you provide a test case I can easily use to validate this bug?

Otherwise, I'll have to close it as presumably fixed, which I would rather not do, as this kind of thing tends to be moved/rehidden rather than fixed. 
Comment 6 Matthias Dieter Wallnöfer 2008-01-07 01:12:51 UTC
Andrew, I don't understand what you mean with a test case.
Please try to find the files and you'll see, that they don't exist anymore or have another form. This is because the registry backend was changed by Jelmer in summer 2007.
I find the bug obsolete. If problems persist also with the new registry backend, the reporter should open new bugs.
Comment 7 Jelmer Vernooij 2008-01-07 08:24:01 UTC
The first bug I'm sure has been fixed. Bug 2 has also been fixed but slightly differently. The bug in reg_diff_apply() has also been fixed I think, because we rely on it in the Python provisioning.

Closing as fixed.