Skip to content

Commit

Permalink
lldpad: pass obuf length to cmd handlers
Browse files Browse the repository at this point in the history
Use safe versions of string copy routines. Arguably in some
cases the input is already validated and guarenteed to be
less then the buffer size. However using safe versions in
these cases is OK and removes any errors or missed cases
during review. Or more likely any possiblitiy of a future
change introducing a potentially unvalidated copy.

To do this the length needed to be passed into the command
handlers.

Signed-off-by: John Fastabend <[email protected]>
  • Loading branch information
John Fastabend committed Jun 17, 2011
1 parent 6fa56ef commit 0ea7641
Show file tree
Hide file tree
Showing 15 changed files with 396 additions and 362 deletions.
82 changes: 41 additions & 41 deletions ctrl_iface.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ static char *hexlist = "0123456789abcdef";
struct clif_cmds {
int cmd;
int (*cmd_handler)(struct clif_data *cd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen);
};

static const struct clif_cmds cmd_tbl[] = {
Expand All @@ -85,10 +85,10 @@ static const struct clif_cmds cmd_tbl[] = {
* Returns a status value. 0 is successful, 1 is an error.
*/
int clif_iface_module(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf)
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen)
{
u32 module_id;
char *cmd_start;
Expand Down Expand Up @@ -119,7 +119,7 @@ int clif_iface_module(struct clif_data *clifd,

if (mod)
return (mod->ops->client_cmd)(clifd, from, fromlen,
cmd_start, cmd_len, rbuf+strlen(rbuf));
cmd_start, cmd_len, rbuf+strlen(rbuf), rlen);
else
return 1;
}
Expand All @@ -129,10 +129,10 @@ int clif_iface_module(struct clif_data *clifd,
* Returns a status value. 0 is successful, 1 is an error.
*/
int clif_iface_cmd_unknown(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf)
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen)
{
return 1;
}
Expand All @@ -141,12 +141,12 @@ int clif_iface_cmd_unknown(struct clif_data *clifd,
* Returns a status value. 0 is successful, 1 is an error.
*/
int clif_iface_ping(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf)
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen)
{
sprintf(rbuf, "%cPONG", PING_CMD);
snprintf(rbuf, rlen, "%cPONG", PING_CMD);

return 0;
}
Expand All @@ -156,10 +156,10 @@ int clif_iface_ping(struct clif_data *clifd,
* Returns a status value. 0 is successful, 1 is an error.
*/
int clif_iface_attach(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf)
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen)
{
struct ctrl_dst *dst;
char *tlv, *str, *tokenize;
Expand Down Expand Up @@ -224,14 +224,14 @@ int clif_iface_attach(struct clif_data *clifd,
free(tlv);

LLDPAD_DBG("CTRL_IFACE monitor attached\n");
sprintf(rbuf, "%c", ATTACH_CMD);
snprintf(rbuf, rlen, "%c", ATTACH_CMD);

return 0;
err_types:
free(tlv);
err_tlv:
LLDPAD_DBG("CTRL_IFACE monitor attach error\n");
sprintf(rbuf, "%c", ATTACH_CMD);
snprintf(rbuf, rlen, "%c", ATTACH_CMD);

return -1;
}
Expand Down Expand Up @@ -274,9 +274,9 @@ int clif_iface_detach(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf)
char *rbuf, int rlen)
{
sprintf(rbuf, "%c", DETACH_CMD);
snprintf(rbuf, rlen, "%c", DETACH_CMD);
return detach_clif_monitor(clifd, from, fromlen);
}

Expand All @@ -288,13 +288,13 @@ int clif_iface_level(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf)
char *rbuf, int rlen)
{
struct ctrl_dst *dst;
char *level;

level = ibuf+1;
sprintf(rbuf, "%c", LEVEL_CMD);
snprintf(rbuf, rlen, "%c", LEVEL_CMD);

LLDPAD_DBG("CTRL_IFACE LEVEL %s", level);

Expand Down Expand Up @@ -323,6 +323,11 @@ static int find_cmd_entry(int cmd)
return (i);
}

/* process_clif_cmd - routine to pass command to correct module routine and
* compute length and status.
*
* Note: ibuf and ilen must be verified by caller.
*/
static void process_clif_cmd( struct clif_data *cd,
struct sockaddr_un *from,
socklen_t fromlen,
Expand All @@ -332,15 +337,11 @@ static void process_clif_cmd( struct clif_data *cd,

/* setup minimum command response message
* status will be updated at end */
sprintf(rbuf, "%c%02x", CMD_RESPONSE, dcb_failed);
*rlen = strlen(rbuf);

if (ilen < 1) {
return;
}

snprintf(rbuf, *rlen, "%c%02x", CMD_RESPONSE, dcb_failed);
status = cmd_tbl[find_cmd_entry((int)ibuf[0])].cmd_handler(
cd, from, fromlen, ibuf, ilen, rbuf+strlen(rbuf));
cd, from, fromlen, ibuf, ilen,
rbuf + strlen(rbuf),
*rlen - strlen(rbuf) - 1);

/* update status and compute final length */
rbuf[CLIF_STAT_OFF] = hexlist[ (status & 0x0f0) >> 4 ];
Expand All @@ -358,12 +359,11 @@ static void ctrl_iface_receive(int sock, void *eloop_ctx,
struct sockaddr_un from;
socklen_t fromlen = sizeof(from);
char *reply;
const int reply_size = MAX_CLIF_MSGBUF;
int reply_len;
int reply_size = MAX_CLIF_MSGBUF;

res = recvfrom(sock, buf, sizeof(buf) - 1, MSG_DONTWAIT,
(struct sockaddr *) &from, &fromlen);
if (res < 0) {
if (res < 1) {
perror("recvfrom(ctrl_iface)");
return;
}
Expand All @@ -377,9 +377,9 @@ static void ctrl_iface_receive(int sock, void *eloop_ctx,
}

memset(reply, 0, reply_size);
process_clif_cmd(clifd, &from, fromlen, buf, res, reply, &reply_len);
process_clif_cmd(clifd, &from, fromlen, buf, res, reply, &reply_size);

sendto(sock, reply, reply_len, 0, (struct sockaddr *) &from, fromlen);
sendto(sock, reply, reply_size, 0, (struct sockaddr *) &from, fromlen);
free(reply);
}

Expand Down
6 changes: 3 additions & 3 deletions include/clif_msgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ struct type_name_info {

struct arg_handlers {
char *arg;
int (* handle_get)(struct cmd *, char *, char *, char *);
int (* handle_set)(struct cmd *, char *, char *, char *);
int (* handle_test)(struct cmd *, char *, char *, char *);
int (*handle_get)(struct cmd *, char *, char *, char *, int);
int (*handle_set)(struct cmd *, char *, char *, char *, int);
int (*handle_test)(struct cmd *, char *, char *, char *, int);
};

#endif
50 changes: 25 additions & 25 deletions include/ctrl_iface.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,36 +53,36 @@ int ctrl_iface_init(struct clif_data *clifd);
int ctrl_iface_register(struct clif_data *clifd);
void ctrl_iface_deinit(struct clif_data *clifd);
void ctrl_iface_send(struct clif_data *clifd, int level, u32 moduleid,
char *buf, size_t len);
char *buf, size_t len);

int clif_iface_attach(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen);
int clif_iface_detach(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen);
int clif_iface_level(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen);
int clif_iface_ping(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen);
int clif_iface_cmd_unknown(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen);
int clif_iface_module(struct clif_data *clifd,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf, int rlen);
#endif /* CTRL_IFACE_H */
2 changes: 1 addition & 1 deletion include/lldp_dcbx.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ void dcbx_ifdown(char *device_name);
struct lldp_module *dcbx_register(void);
void dcbx_unregister(struct lldp_module *);
int dcbx_clif_cmd(void *, struct sockaddr_un *,
socklen_t , char *, int, char *);
socklen_t , char *, int, char *, int);

#endif /* _LLDP_DCBX_H */
4 changes: 1 addition & 3 deletions include/lldp_mand.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#define _LLDP_MAND_H

#include "lldp_mod.h"
#include "lldp_mand_cmds.h"

#define LLDP_MOD_MAND 1

Expand All @@ -51,7 +52,4 @@ void mand_unregister(struct lldp_module *mod);
struct packed_tlv *mand_gettlv(struct port *port);
void mand_ifdown(char *);
void mand_ifup(char *);
int mand_clif_cmd(void *, struct sockaddr_un *,
socklen_t , char *, int, char *);

#endif /* _LLDP_MAND_H */
2 changes: 1 addition & 1 deletion include/lldp_mand_cmds.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ int mand_clif_cmd(void *data,
struct sockaddr_un *from,
socklen_t fromlen,
char *ibuf, int ilen,
char *rbuf);
char *rbuf, int rlen);

#endif
2 changes: 1 addition & 1 deletion include/lldp_mod.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ struct lldp_mod_ops {
int (* client_cmd)(void *data,
struct sockaddr_un *from,
socklen_t fromlen, char *ibuf,
int ilen, char *rbuf);
int ilen, char *rbuf, int rlen);
int (* print_tlv)(u32, u16, char *);
u32 (* lookup_tlv_name)(char *);
int (* print_help)();
Expand Down
Loading

0 comments on commit 0ea7641

Please sign in to comment.