Bug 81 - climessage.c routines have the potential to block, especially between "send" and "receive".
climessage.c routines have the potential to block, especially between "send" ...
Status: CLOSED FIXED
Product: Samba 3.0
Classification: Unclassified
Component: smbclient
3.0.0preX
Other other
: P3 normal
: none
Assigned To: Gerald (Jerry) Carter
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-05-13 05:52 UTC by Gerald (Jerry) Carter
Modified: 2005-08-24 10:24 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gerald (Jerry) Carter 2003-05-13 05:52:04 UTC
Enclosed is a patch to fix a blocking problem in the family of routines:
   cli_message_start()
   cli_message_text()
   cli_message_end()

As written each of these is:

cli_message_X() {
   inline_build_message_X;
   cli_send_smb(cli);
   cli_receive_smb(cli);
}

This has the potential to block, especially between "send" and "receive".

If the caller is something simple and synchronous, such as the "smbclient"
program, there is not really a problem.  But if one builds a more
sophisticated caller, which requires asynchronous, polling I/O, then this
blocking could be unpleasant.  (A potential example is "write"/"wall"
functionality discussed in recent years, which might even be part of a
future "smbd".)

The idea of the patch is to put the intricate, non-blocking
"inline_build_message_X" code as separate subroutines, so each routine now
becomes a pair:

cli_message_X_build() {
   inline_build_message_X;
}

cli_message_X() {
   cli_message_X_build();
   cli_send_smb(cli);
   cli_receive_smb(cli);
}


The change is entirely self-contained and makes (or should make!) no
difference whatsoever to existing applications and linking (such as the
"smbclient" program which continues to use the unchanged "cli_message_X()"
interface.

The gain is allowing the writing of newer applications which can call the
common-code "cli_message_X_build()" routines, and then do their own,
async, calling of "cli_send_smb()" and "cli_receive_smb()".

The patch below is against 3.0alpha23:

================================ snip ===============================
--- climessage.c.orig	Mon Mar 31 03:28:29 2003
+++ climessage.c	Wed Apr 23 18:00:39 2003
@@ -26,12 +26,11 @@
 /****************************************************************************
 start a message sequence
 ****************************************************************************/
-BOOL cli_message_start(struct cli_state *cli, char *host, char *username,
-			      int *grp)
+int cli_message_start_build(struct cli_state *cli, char *host, char *username)
 {
 	char *p;

-	/* send a SMBsendstrt command */
+	/* construct a SMBsendstrt command */
 	memset(cli->outbuf,'\0',smb_size);
 	set_message(cli->outbuf,0,0,True);
 	SCVAL(cli->outbuf,smb_com,SMBsendstrt);
@@ -45,6 +44,14 @@
 	p += clistr_push(cli, p, host, -1, STR_TERMINATE);

 	cli_setup_bcc(cli, p);
+
+	return(PTR_DIFF(p, cli->outbuf));
+}
+
+BOOL cli_message_start(struct cli_state *cli, char *host, char *username,
+			      int *grp)
+{
+	cli_message_start_build(cli, host, username);

 	cli_send_smb(cli);

@@ -63,7 +70,7 @@
 /****************************************************************************
 send a message
 ****************************************************************************/
-BOOL cli_message_text(struct cli_state *cli, char *msg, int len, int grp)
+int cli_message_text_build(struct cli_state *cli, char *msg, int len, int grp)
 {
 	char *msgdos;
 	int lendos;
@@ -93,6 +100,14 @@
 	}

 	cli_setup_bcc(cli, p);
+
+	return(PTR_DIFF(p, cli->outbuf));
+}
+
+BOOL cli_message_text(struct cli_state *cli, char *msg, int len, int grp)
+{
+	cli_message_text_build(cli, msg, len, grp);
+
 	cli_send_smb(cli);

 	if (!cli_receive_smb(cli)) {
@@ -107,8 +122,10 @@
 /****************************************************************************
 end a message
 ****************************************************************************/
-BOOL cli_message_end(struct cli_state *cli, int grp)
+int cli_message_end_build(struct cli_state *cli, int grp)
 {
+	char *p;
+
 	memset(cli->outbuf,'\0',smb_size);
 	set_message(cli->outbuf,1,0,True);
 	SCVAL(cli->outbuf,smb_com,SMBsendend);
@@ -117,7 +134,16 @@
 	SSVAL(cli->outbuf,smb_vwv0,grp);

 	cli_setup_packet(cli);
-
+
+	p = smb_buf(cli->outbuf);
+
+	return(PTR_DIFF(p, cli->outbuf));
+}
+
+BOOL cli_message_end(struct cli_state *cli, int grp)
+{
+	cli_message_end_build(cli, grp);
+
 	cli_send_smb(cli);

 	if (!cli_receive_smb(cli)) {
================================ snip ===============================

-- 

:  David Lee                                I.T. Service          :
:  Systems Programmer                       Computer Centre       :
:                                           University of Durham  :
:  http://www.dur.ac.uk/t.d.lee/            South Road            :
:                                           Durham                :
:  Phone: +44 191 334 2752                  U.K.                  :
Comment 1 Gerald (Jerry) Carter 2003-06-06 16:08:35 UTC
i'll buy it.  Applied
Comment 2 Gerald (Jerry) Carter 2005-02-07 07:54:46 UTC
originally reported against 3.0alpha23.  Bugzilla spring cleaning.
Comment 3 Gerald (Jerry) Carter 2005-08-24 10:24:44 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.