Skip to content

Commit 18c3546

Browse files
Hakon-Buggearon-silverton
authored andcommitted
ibacm: Copy correct number of address bytes before calling provider
In acm_ep_insert_addr() an attempt to zero out the tmp address buffer is performed. But the subsequent memcpy(), which uses the supplied addr_len as argument, copies the whole shebang. This implies that the provider is called with an address with arbitrary data padded. This leads to a false mis-compare in the default provider's binary tree lookup. Here is the stack trace and dump of the address buffer from gdb (edited for better brevity): (gdb) where #0 acmp_compare_dest (dest1=0x18c46a8, dest2=0x18c5d70) at prov/acmp/src/acmp.c:289 linux-rdma#1 tfind () from /lib64/libc.so.6 linux-rdma#2 acmp_get_dest () at prov/acmp/src/acmp.c:336 linux-rdma#3 acmp_acquire_dest () at prov/acmp/src/acmp.c:379 linux-rdma#4 acmp_add_addr () at prov/acmp/src/acmp.c:2385 linux-rdma#5 acm_ep_insert_addr (..., addr_len=addr_len@entry=64, ...) at src/acm.c:2044 linux-rdma#6 acm_ep_insert_addr (..., addr_len=64, ...) at src/acm.c:1325 linux-rdma#7 acm_add_ep_ip (ip_str=0x7ffeeda298e0 "192.168.200.200", ...) at src/acm.c:1326 linux-rdma#8 acm_ipnl_handler () at src/acm.c:1453 linux-rdma#9 acm_server () at src/acm.c:1884 linux-rdma#10 main () at src/acm.c:3245 (gdb) x/20u dest1 0x18c46a8: 192 168 200 200 155 127 0 0 0x18c46b0: 95 184 77 105 155 127 0 0 0x18c46b8: 0 0 64 49 (gdb) x/20u dest2 0x18c5d70: 192 168 200 200 0 0 0 0 0x18c5d78: 0 0 0 0 0 0 0 0 0x18c5d80: 0 0 0 0 The fix is to use the real length of the address in the memcpy() in acm_ep_insert_addr(). This is derived from the addr_type. Hence, we can re-factor and remove the addr_len from the call stack. Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com> Reviewed-by: Mark Haywood <mark.haywood@oracle.com> Orabug: 29037270 (cherry picked from commit c73f5d7) cherry-pick-repo=linux-rdma/rdma-core.git unmodified-from-upstream: c73f5d7 Signed-off-by: Mark Haywood <mark.haywood@oracle.com> Signed-off-by: Aron Silverton <aron.silverton@oracle.com>
1 parent da9eb91 commit 18c3546

File tree

4 files changed

+24
-22
lines changed

4 files changed

+24
-22
lines changed

ibacm/src/acm.c

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ static struct acmc_device *
197197
acm_get_device_from_gid(union ibv_gid *sgid, uint8_t *port);
198198
static struct acmc_ep *acm_find_ep(struct acmc_port *port, uint16_t pkey);
199199
static int acm_ep_insert_addr(struct acmc_ep *ep, const char *name, uint8_t *addr,
200-
size_t addr_len, uint8_t addr_type);
200+
uint8_t addr_type);
201201
static void acm_event_handler(struct acmc_device *dev);
202202
static int acm_nl_send(int sock, struct acm_msg *msg);
203203

@@ -1326,7 +1326,7 @@ static void acm_add_ep_ip(char *ifname, struct acm_ep_addr_data *data, char *ip_
13261326
ep = acm_find_ep(&dev->port[port_num - 1], pkey);
13271327
if (ep) {
13281328
if (acm_ep_insert_addr(ep, ip_str, data->info.addr,
1329-
sizeof data->info.addr, data->type))
1329+
data->type))
13301330
acm_log(0, "Failed to add '%s' to EP\n", ip_str);
13311331
} else {
13321332
acm_log(0, "Failed to add '%s' no EP for pkey\n", ip_str);
@@ -1370,7 +1370,7 @@ static int acm_ipnl_create(void)
13701370
}
13711371

13721372
static void acm_ip_iter_cb(char *ifname, union ibv_gid *gid, uint16_t pkey,
1373-
uint8_t addr_type, uint8_t *addr, size_t addr_len,
1373+
uint8_t addr_type, uint8_t *addr,
13741374
char *ip_str, void *ctx)
13751375
{
13761376
int ret = EINVAL;
@@ -1383,7 +1383,7 @@ static void acm_ip_iter_cb(char *ifname, union ibv_gid *gid, uint16_t pkey,
13831383
if (dev) {
13841384
ep = acm_find_ep(&dev->port[port_num - 1], pkey);
13851385
if (ep)
1386-
ret = acm_ep_insert_addr(ep, ip_str, addr, addr_len, addr_type);
1386+
ret = acm_ep_insert_addr(ep, ip_str, addr, addr_type);
13871387
}
13881388

13891389
if (ret) {
@@ -2009,16 +2009,13 @@ static FILE *acm_open_addr_file(void)
20092009

20102010
static int
20112011
acm_ep_insert_addr(struct acmc_ep *ep, const char *name, uint8_t *addr,
2012-
size_t addr_len, uint8_t addr_type)
2012+
uint8_t addr_type)
20132013
{
20142014
int i, ret = -1;
20152015
uint8_t tmp[ACM_MAX_ADDRESS];
20162016

2017-
if (addr_len > ACM_MAX_ADDRESS)
2018-
return EINVAL;
2019-
20202017
memset(tmp, 0, sizeof tmp);
2021-
memcpy(tmp, addr, addr_len);
2018+
memcpy(tmp, addr, acm_addr_len(addr_type));
20222019

20232020
if (!acm_addr_lookup(&ep->endpoint, addr, addr_type)) {
20242021
for (i = 0; (i < MAX_EP_ADDR) &&
@@ -2079,7 +2076,7 @@ acm_get_device_from_gid(union ibv_gid *sgid, uint8_t *port)
20792076
}
20802077

20812078
static void acm_ep_ip_iter_cb(char *ifname, union ibv_gid *gid, uint16_t pkey,
2082-
uint8_t addr_type, uint8_t *addr, size_t addr_len,
2079+
uint8_t addr_type, uint8_t *addr,
20832080
char *ip_str, void *ctx)
20842081
{
20852082
uint8_t port_num;
@@ -2091,7 +2088,7 @@ static void acm_ep_ip_iter_cb(char *ifname, union ibv_gid *gid, uint16_t pkey,
20912088
&& ep->port->port.port_num == port_num &&
20922089
/* pkey retrieved from ipoib has always full mmbr bit set */
20932090
(ep->endpoint.pkey | IB_PKEY_FULL_MEMBER) == pkey) {
2094-
if (!acm_ep_insert_addr(ep, ip_str, addr, addr_len, addr_type)) {
2091+
if (!acm_ep_insert_addr(ep, ip_str, addr, addr_type)) {
20952092
acm_log(0, "Added %s %s %d 0x%x from %s\n", ip_str,
20962093
dev->device.verbs->device->name, port_num, pkey,
20972094
ifname);
@@ -2113,7 +2110,6 @@ static int acm_assign_ep_names(struct acmc_ep *ep)
21132110
uint16_t pkey;
21142111
uint8_t addr[ACM_MAX_ADDRESS], type;
21152112
int port;
2116-
size_t addr_len;
21172113

21182114
dev_name = ep->port->dev->device.verbs->device->name;
21192115
acm_log(1, "device %s, port %d, pkey 0x%x\n",
@@ -2140,18 +2136,15 @@ static int acm_assign_ep_names(struct acmc_ep *ep)
21402136
continue;
21412137
}
21422138
type = ACM_ADDRESS_IP;
2143-
addr_len = 4;
21442139
} else if (inet_pton(AF_INET6, name, addr) > 0) {
21452140
if (!support_ips_in_addr_cfg) {
21462141
acm_log(0, "ERROR - IP's are not configured to be read from ibacm_addr.cfg\n");
21472142
continue;
21482143
}
21492144
type = ACM_ADDRESS_IP6;
2150-
addr_len = 16;
21512145
} else {
21522146
type = ACM_ADDRESS_NAME;
2153-
addr_len = strlen(name);
2154-
memcpy(addr, name, addr_len);
2147+
memcpy(addr, name, strlen(name));
21552148
}
21562149

21572150
if (strcasecmp(pkey_str, "default")) {
@@ -2167,7 +2160,7 @@ static int acm_assign_ep_names(struct acmc_ep *ep)
21672160
(ep->port->port.port_num == (uint8_t) port) &&
21682161
(ep->endpoint.pkey == pkey)) {
21692162
acm_log(1, "assigning %s\n", name);
2170-
if (acm_ep_insert_addr(ep, name, addr, addr_len, type)) {
2163+
if (acm_ep_insert_addr(ep, name, addr, type)) {
21712164
acm_log(1, "maximum number of names assigned to EP\n");
21722165
break;
21732166
}

ibacm/src/acm_util.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx)
133133
union ibv_gid sgid;
134134
uint8_t addr_type;
135135
uint8_t addr[ACM_MAX_ADDRESS];
136-
size_t addr_len;
137136
char *alias_sep;
138137

139138
s = socket(family, SOCK_DGRAM, 0);
@@ -180,7 +179,6 @@ int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx)
180179
addr_type = ACM_ADDRESS_IP;
181180
memcpy(&addr, &((struct sockaddr_in *) &ifr[i].ifr_addr)->sin_addr,
182181
sizeof addr);
183-
addr_len = 4;
184182
inet_ntop(ifr[i].ifr_addr.sa_family,
185183
&((struct sockaddr_in *) &ifr[i].ifr_addr)->sin_addr,
186184
ip_str, sizeof ip_str);
@@ -189,7 +187,6 @@ int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx)
189187
addr_type = ACM_ADDRESS_IP6;
190188
memcpy(&addr, &((struct sockaddr_in6 *) &ifr[i].ifr_addr)->sin6_addr,
191189
sizeof addr);
192-
addr_len = ACM_MAX_ADDRESS;
193190
inet_ntop(ifr[i].ifr_addr.sa_family,
194191
&((struct sockaddr_in6 *) &ifr[i].ifr_addr)->sin6_addr,
195192
ip_str, sizeof ip_str);
@@ -215,7 +212,7 @@ int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx)
215212
if (ret)
216213
continue;
217214

218-
cb(ifr[i].ifr_name, &sgid, pkey, addr_type, addr, addr_len, ip_str, ctx);
215+
cb(ifr[i].ifr_name, &sgid, pkey, addr_type, addr, ip_str, ctx);
219216
}
220217
ret = 0;
221218

ibacm/src/acm_util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ int acm_if_get_pkey(char *ifname, uint16_t *pkey);
4949
int acm_if_get_sgid(char *ifname, union ibv_gid *sgid);
5050

5151
typedef void (*acm_if_iter_cb)(char *ifname, union ibv_gid *gid, uint16_t pkey,
52-
uint8_t addr_type, uint8_t *addr, size_t addr_len,
52+
uint8_t addr_type, uint8_t *addr,
5353
char *ip_str, void *ctx);
5454
int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx);
5555

oracle/rdma-core.spec

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,18 @@ rm -f %{buildroot}/%{_sbindir}/srp_daemon.sh
606606
- libibverbs: Fix compiler warnings in examples/frc_pingpong.c (Mark Haywood) [Orabug: 28769907]
607607
- librdmacm: Fix compiler warnings in librdmacm/cma.c (Mark Haywood) [Orabug: 28794664]
608608
- libibverbs: Fix compiler warnings in examples/shpd_pingpong.c (Mark Haywood) [Orabug: 28802228]
609+
- ibacm: Log using RCF3339 timestamps (Matt Ezell) [Orabug: 29037074]
610+
- ibacm: remove endpoint IP address from provider when address deleted (Mark Haywood) [Orabug: 28845883]
611+
- ibacm: Check return value when deleting a cache entry (Håkon Bugge) [Orabug: 29037101]
612+
- ibacm: Flush cache in provider when local address is removed (Håkon Bugge) [Orabug: 29037124]
613+
- ibacm: Remove trailing blanks (Håkon Bugge) [Orabug: 29037144]
614+
- ibacm: Fix proper return value from ib_acme (Håkon Bugge) [Orabug: 29037153]
615+
- ibacm: Handle rereg event coming before link active event (Håkon Bugge) [Orabug: 29037189]
616+
- ibacm: Use helper functions and existing defines to avoid literal use (Håkon Bugge) [Orabug: 29026611]
617+
- ibacm: Unable to assign EP name for limited pkey (Håkon Bugge) [Orabug: 29037216]
618+
- ibacm: Fix improper refcnt (Håkon Bugge) [Orabug: 29037237]
619+
- ibacm: Fix incorrect length used in address comparisons (Håkon Bugge) [Orabug: 29037253]
620+
- ibacm: Copy correct number of address bytes before calling provider (Håkon Bugge) [Orabug: 29037270]
609621

610622
* Fri Nov 09 2018 Aron Silverton <aron.silverton@oracle.com> - 5:17.1-1.0.6
611623
- oracle/spec: Change "vos" to "ora" and update summaries and descriptions [Orabug: 29128747]

0 commit comments

Comments
 (0)