[Linux-cifsd-devel] [PATCH] cifsd: add v4 dos attribute structure

[Stefan Metzmacher]

Hi Namjae,

>>> @@ -153,17 +155,27 @@ int ndr_encode_dos_attr(struct ndr *n, struct
>>> xattr_dos_attrib *da)
>>>  	if (!n->data)
>>>  		return -ENOMEM;
>>>
>>> -	snprintf(hex_attr, 10, "0x%x", da->attr);
>>> -	ndr_write_string(n, hex_attr, strlen(hex_attr));
>>> +	if (da->version == 3) {
>>> +		snprintf(hex_attr, 10, "0x%x", da->attr);
>>> +		ndr_write_string(n, hex_attr, strlen(hex_attr));
>>> +	} else {
>>> +		ndr_write_int16(n, 0);
>>> +		flags |= XATTR_DOSINFO_ITIME;
>>> +	}
>>
>> This looks wrong, the hex string is present in all versions.
> When I dump xattr DOSATTRIB, null terminator string(zero bytes) seems
> to be stored in there.
> I checked the following function related to hex string in samba code.
> there seems be no switch case for version 4. Am I missing something ?
> 
> static char *ndr_compat_xattr_attrib_hex(TALLOC_CTX *mem_ctx,
>                                         const struct xattr_DOSATTRIB *r)
> {
>         char *attrib_hex = NULL;
> 
>         switch (r->version) {
>         case 0xFFFF:
>                 attrib_hex = talloc_asprintf(mem_ctx, "0x%x",
>                                         r->info.compatinfoFFFF.attrib);
>                 break;
>         case 1:
>                 attrib_hex = talloc_asprintf(mem_ctx, "0x%x",
>                                         r->info.info1.attrib);
>                 break;
>         case 2:
>                 attrib_hex = talloc_asprintf(mem_ctx, "0x%x",
>                                         r->info.oldinfo2.attrib);
>                 break;
>         case 3:
>                 if (!(r->info.info3.valid_flags & XATTR_DOSINFO_ATTRIB)) {
>                         attrib_hex = talloc_strdup(mem_ctx, "");
>                         break;
>                 }
>                 attrib_hex = talloc_asprintf(mem_ctx, "0x%x",
>                                         r->info.info3.attrib);
>                 break;
>         default:
>                 attrib_hex = talloc_strdup(mem_ctx, "");
>                 break;
>         }

Hmm, you're right, but this wasn't really intended :-(

Given that all maintained versions produced version 4 with an empty string,
we could just leave it that way, it's very unlikely that someone tries to
downgrade from Samba 4.11 back to 3.4.

Ralph, Jeremy, what do you say?

>> And the create_time should only be updated, when the client provided a valid
>> value.
> Hm.. the meaning "the client provided" is saying
> FILE_BASIC_INFORMATION of smb2 set info ? I am wondering why similar
> two creation time are handled.

See MS-FSA 2.4.7 FileBasicInformation, -1 and -2 are special values,
so far I've only seen -1 in the wild, it means the client can update
only a single timestamp and leaves the others unchanged.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20210216/0492cce3/signature.sig>