Bug 12479 - cli_ftruncate fails with an SMB2+ connection
cli_ftruncate fails with an SMB2+ connection
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient
4.4.8
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-21 20:20 UTC by Paul Smedley
Modified: 2017-01-11 08:02 UTC (History)
2 users (show)

See Also:


Attachments
test patch for master (3.44 KB, patch)
2016-12-21 22:14 UTC, Jeremy Allison
no flags Details
Doh - fixed patch including size. (3.52 KB, patch)
2016-12-21 22:50 UTC, Jeremy Allison
no flags Details
git-am fix for master. (9.50 KB, patch)
2017-01-04 00:28 UTC, Jeremy Allison
no flags Details
git-am fix for 4.5.next. (9.77 KB, patch)
2017-01-05 19:31 UTC, Jeremy Allison
uri: review+
Details
git-am fix for 4.4.next (9.91 KB, patch)
2017-01-05 19:31 UTC, Jeremy Allison
uri: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Smedley 2016-12-21 20:20:41 UTC
Whilst developing my OS/2 Samba client, I discovered that cli_ftruncate is failing with a SMB2 or above connection.

cli_error returns 22 (EINVAL).

From: Jeremy on samba-technical

"Good catch - looks like we missed converting cli_ftruncate() to
call SMB2+ under the covers.

Inside cli_getatr() for example we have:

        if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
                return cli_smb2_getatr(cli,
                                        fname,
                                        attr,
                                        size,
                                        write_time);
        }

We need the same adding for cli_ftruncate() and a callout
to a cli_smb2_ftruncate() which needs adding inside
source3/libsmb/cli_smb2_fnum.c."
Comment 1 Jeremy Allison 2016-12-21 22:14:34 UTC
Created attachment 12775 [details]
test patch for master
Comment 2 Jeremy Allison 2016-12-21 22:50:30 UTC
Created attachment 12776 [details]
Doh - fixed patch including size.

Doh - sorry - ignore previous patch. I forgot to add the 'size'
argument to cli_smb2_ftruncate() (would always have truncated
to zero).
Comment 3 Paul Smedley 2016-12-22 02:50:33 UTC
Test fix appears to work fine here! Thanks!
Comment 4 Paul Smedley 2017-01-02 09:00:46 UTC
Any idea when this fix will get committed?
Comment 5 Jeremy Allison 2017-01-04 00:28:12 UTC
Created attachment 12792 [details]
git-am fix for master.

Contains regression test.
Comment 6 Jeremy Allison 2017-01-05 19:31:25 UTC
Created attachment 12800 [details]
git-am fix for 4.5.next.

Back-ported from master.
Comment 7 Jeremy Allison 2017-01-05 19:31:56 UTC
Created attachment 12801 [details]
git-am fix for 4.4.next

Back-ported from master.
Comment 8 Uri Simchoni 2017-01-06 19:04:10 UTC
Comment on attachment 12800 [details]
git-am fix for 4.5.next.

Oops, just spotted something (in master too, in SMB1 code too) - there's no check on the talloc_stackframe() result.

Jeremy, what do you think? Isn't it our coding convention to check tallocs? Maybe add it as a followup patch and backport that too?
Comment 9 Jeremy Allison 2017-01-06 20:08:40 UTC
talloc_stackframe() calls abort() by design if it fails. It's so integral that it's treated (rightly IMHO) as a catastrophic failure.
Comment 10 Uri Simchoni 2017-01-06 20:11:36 UTC
Assigning to Karolin for inclusion in 4.4.next and 4.5.next.
Thanks,
Uri.
Comment 11 Karolin Seeger 2017-01-09 08:21:22 UTC
(In reply to Uri Simchoni from comment #10)
Pushed to autobild-v4-{5,4}-test.
Comment 12 Karolin Seeger 2017-01-11 08:02:15 UTC
(In reply to Karolin Seeger from comment #11)
Pushed to both branches.
Closing out bug report.

Thanks!