Bug 12479 - cli_ftruncate fails with an SMB2+ connection
Summary: cli_ftruncate fails with an SMB2+ connection
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.4.8
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

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!