Commit b26ddd813031666293c95e84c997eb8b1f97bd38

Authored by Neil Horman
Committed by David S. Miller
1 parent 1a9408355e

sctp: Clean up type-punning in sctp_cmd_t union

Lots of points in the sctp_cmd_interpreter function treat the sctp_cmd_t arg as
a void pointer, even though they are written as various other types.  Theres no
need for this as doing so just leads to possible type-punning issues that could
cause crashes, and if we remain type-consistent we can actually just remove the
void * member of the union entirely.

Change Notes:

v2)
	* Dropped chunk that modified SCTP_NULL to create a marker pattern
	 should anyone try to use a SCTP_NULL() assigned sctp_arg_t, Assigning
	 to .zero provides the same effect and should be faster, per Vlad Y.

v3)
	* Reverted part of V2, opting to use memset instead of .zero, so that
	 the entire union is initalized thus avoiding the i164 speculative load
	 problems previously encountered, per Dave M..  Also rewrote
	 SCTP_[NO]FORCE so as to use common infrastructure a little more

Signed-off-by: Neil Horman <nhorman@tuxdriver.com
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: linux-sctp@vger.kernel.org
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 4 changed files with 46 additions and 42 deletions Side-by-side Diff

include/net/sctp/command.h
... ... @@ -130,8 +130,6 @@
130 130 __be16 err;
131 131 sctp_state_t state;
132 132 sctp_event_timeout_t to;
133   - unsigned long zero;
134   - void *ptr;
135 133 struct sctp_chunk *chunk;
136 134 struct sctp_association *asoc;
137 135 struct sctp_transport *transport;
138 136  
... ... @@ -154,23 +152,15 @@
154 152 * which takes an __s32 and returns a sctp_arg_t containing the
155 153 * __s32. So, after foo = SCTP_I32(arg), foo.i32 == arg.
156 154 */
157   -static inline sctp_arg_t SCTP_NULL(void)
158   -{
159   - sctp_arg_t retval; retval.ptr = NULL; return retval;
160   -}
161   -static inline sctp_arg_t SCTP_NOFORCE(void)
162   -{
163   - sctp_arg_t retval = {.zero = 0UL}; retval.i32 = 0; return retval;
164   -}
165   -static inline sctp_arg_t SCTP_FORCE(void)
166   -{
167   - sctp_arg_t retval = {.zero = 0UL}; retval.i32 = 1; return retval;
168   -}
169 155  
170 156 #define SCTP_ARG_CONSTRUCTOR(name, type, elt) \
171 157 static inline sctp_arg_t \
172 158 SCTP_## name (type arg) \
173   -{ sctp_arg_t retval = {.zero = 0UL}; retval.elt = arg; return retval; }
  159 +{ sctp_arg_t retval;\
  160 + memset(&retval, 0, sizeof(sctp_arg_t));\
  161 + retval.elt = arg;\
  162 + return retval;\
  163 +}
174 164  
175 165 SCTP_ARG_CONSTRUCTOR(I32, __s32, i32)
176 166 SCTP_ARG_CONSTRUCTOR(U32, __u32, u32)
... ... @@ -181,7 +171,6 @@
181 171 SCTP_ARG_CONSTRUCTOR(PERR, __be16, err) /* protocol error */
182 172 SCTP_ARG_CONSTRUCTOR(STATE, sctp_state_t, state)
183 173 SCTP_ARG_CONSTRUCTOR(TO, sctp_event_timeout_t, to)
184   -SCTP_ARG_CONSTRUCTOR(PTR, void *, ptr)
185 174 SCTP_ARG_CONSTRUCTOR(CHUNK, struct sctp_chunk *, chunk)
186 175 SCTP_ARG_CONSTRUCTOR(ASOC, struct sctp_association *, asoc)
187 176 SCTP_ARG_CONSTRUCTOR(TRANSPORT, struct sctp_transport *, transport)
... ... @@ -191,6 +180,23 @@
191 180 SCTP_ARG_CONSTRUCTOR(PACKET, struct sctp_packet *, packet)
192 181 SCTP_ARG_CONSTRUCTOR(SACKH, sctp_sackhdr_t *, sackh)
193 182 SCTP_ARG_CONSTRUCTOR(DATAMSG, struct sctp_datamsg *, msg)
  183 +
  184 +static inline sctp_arg_t SCTP_FORCE(void)
  185 +{
  186 + return SCTP_I32(1);
  187 +}
  188 +
  189 +static inline sctp_arg_t SCTP_NOFORCE(void)
  190 +{
  191 + return SCTP_I32(0);
  192 +}
  193 +
  194 +static inline sctp_arg_t SCTP_NULL(void)
  195 +{
  196 + sctp_arg_t retval;
  197 + memset(&retval, 0, sizeof(sctp_arg_t));
  198 + return retval;
  199 +}
194 200  
195 201 typedef struct {
196 202 sctp_arg_t obj;
include/net/sctp/ulpqueue.h
... ... @@ -72,7 +72,7 @@
72 72 void sctp_ulpq_renege(struct sctp_ulpq *, struct sctp_chunk *, gfp_t);
73 73  
74 74 /* Perform partial delivery. */
75   -void sctp_ulpq_partial_delivery(struct sctp_ulpq *, struct sctp_chunk *, gfp_t);
  75 +void sctp_ulpq_partial_delivery(struct sctp_ulpq *, gfp_t);
76 76  
77 77 /* Abort the partial delivery. */
78 78 void sctp_ulpq_abort_pd(struct sctp_ulpq *, gfp_t);
net/sctp/sm_sideeffect.c
... ... @@ -1268,14 +1268,14 @@
1268 1268 sctp_outq_uncork(&asoc->outqueue);
1269 1269 local_cork = 0;
1270 1270 }
1271   - asoc = cmd->obj.ptr;
  1271 + asoc = cmd->obj.asoc;
1272 1272 /* Register with the endpoint. */
1273 1273 sctp_endpoint_add_asoc(ep, asoc);
1274 1274 sctp_hash_established(asoc);
1275 1275 break;
1276 1276  
1277 1277 case SCTP_CMD_UPDATE_ASSOC:
1278   - sctp_assoc_update(asoc, cmd->obj.ptr);
  1278 + sctp_assoc_update(asoc, cmd->obj.asoc);
1279 1279 break;
1280 1280  
1281 1281 case SCTP_CMD_PURGE_OUTQUEUE:
... ... @@ -1315,7 +1315,7 @@
1315 1315 break;
1316 1316  
1317 1317 case SCTP_CMD_PROCESS_FWDTSN:
1318   - sctp_cmd_process_fwdtsn(&asoc->ulpq, cmd->obj.ptr);
  1318 + sctp_cmd_process_fwdtsn(&asoc->ulpq, cmd->obj.chunk);
1319 1319 break;
1320 1320  
1321 1321 case SCTP_CMD_GEN_SACK:
... ... @@ -1331,7 +1331,7 @@
1331 1331 case SCTP_CMD_PROCESS_SACK:
1332 1332 /* Process an inbound SACK. */
1333 1333 error = sctp_cmd_process_sack(commands, asoc,
1334   - cmd->obj.ptr);
  1334 + cmd->obj.chunk);
1335 1335 break;
1336 1336  
1337 1337 case SCTP_CMD_GEN_INIT_ACK:
1338 1338  
... ... @@ -1352,15 +1352,15 @@
1352 1352 * layer which will bail.
1353 1353 */
1354 1354 error = sctp_cmd_process_init(commands, asoc, chunk,
1355   - cmd->obj.ptr, gfp);
  1355 + cmd->obj.init, gfp);
1356 1356 break;
1357 1357  
1358 1358 case SCTP_CMD_GEN_COOKIE_ECHO:
1359 1359 /* Generate a COOKIE ECHO chunk. */
1360 1360 new_obj = sctp_make_cookie_echo(asoc, chunk);
1361 1361 if (!new_obj) {
1362   - if (cmd->obj.ptr)
1363   - sctp_chunk_free(cmd->obj.ptr);
  1362 + if (cmd->obj.chunk)
  1363 + sctp_chunk_free(cmd->obj.chunk);
1364 1364 goto nomem;
1365 1365 }
1366 1366 sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
1367 1367  
... ... @@ -1369,9 +1369,9 @@
1369 1369 /* If there is an ERROR chunk to be sent along with
1370 1370 * the COOKIE_ECHO, send it, too.
1371 1371 */
1372   - if (cmd->obj.ptr)
  1372 + if (cmd->obj.chunk)
1373 1373 sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
1374   - SCTP_CHUNK(cmd->obj.ptr));
  1374 + SCTP_CHUNK(cmd->obj.chunk));
1375 1375  
1376 1376 if (new_obj->transport) {
1377 1377 new_obj->transport->init_sent_count++;
1378 1378  
1379 1379  
1380 1380  
... ... @@ -1417,18 +1417,18 @@
1417 1417 case SCTP_CMD_CHUNK_ULP:
1418 1418 /* Send a chunk to the sockets layer. */
1419 1419 SCTP_DEBUG_PRINTK("sm_sideff: %s %p, %s %p.\n",
1420   - "chunk_up:", cmd->obj.ptr,
  1420 + "chunk_up:", cmd->obj.chunk,
1421 1421 "ulpq:", &asoc->ulpq);
1422   - sctp_ulpq_tail_data(&asoc->ulpq, cmd->obj.ptr,
  1422 + sctp_ulpq_tail_data(&asoc->ulpq, cmd->obj.chunk,
1423 1423 GFP_ATOMIC);
1424 1424 break;
1425 1425  
1426 1426 case SCTP_CMD_EVENT_ULP:
1427 1427 /* Send a notification to the sockets layer. */
1428 1428 SCTP_DEBUG_PRINTK("sm_sideff: %s %p, %s %p.\n",
1429   - "event_up:",cmd->obj.ptr,
  1429 + "event_up:",cmd->obj.ulpevent,
1430 1430 "ulpq:",&asoc->ulpq);
1431   - sctp_ulpq_tail_event(&asoc->ulpq, cmd->obj.ptr);
  1431 + sctp_ulpq_tail_event(&asoc->ulpq, cmd->obj.ulpevent);
1432 1432 break;
1433 1433  
1434 1434 case SCTP_CMD_REPLY:
1435 1435  
... ... @@ -1438,12 +1438,12 @@
1438 1438 local_cork = 1;
1439 1439 }
1440 1440 /* Send a chunk to our peer. */
1441   - error = sctp_outq_tail(&asoc->outqueue, cmd->obj.ptr);
  1441 + error = sctp_outq_tail(&asoc->outqueue, cmd->obj.chunk);
1442 1442 break;
1443 1443  
1444 1444 case SCTP_CMD_SEND_PKT:
1445 1445 /* Send a full packet to our peer. */
1446   - packet = cmd->obj.ptr;
  1446 + packet = cmd->obj.packet;
1447 1447 sctp_packet_transmit(packet);
1448 1448 sctp_ootb_pkt_free(packet);
1449 1449 break;
... ... @@ -1480,7 +1480,7 @@
1480 1480 break;
1481 1481  
1482 1482 case SCTP_CMD_SETUP_T2:
1483   - sctp_cmd_setup_t2(commands, asoc, cmd->obj.ptr);
  1483 + sctp_cmd_setup_t2(commands, asoc, cmd->obj.chunk);
1484 1484 break;
1485 1485  
1486 1486 case SCTP_CMD_TIMER_START_ONCE:
... ... @@ -1514,7 +1514,7 @@
1514 1514 break;
1515 1515  
1516 1516 case SCTP_CMD_INIT_CHOOSE_TRANSPORT:
1517   - chunk = cmd->obj.ptr;
  1517 + chunk = cmd->obj.chunk;
1518 1518 t = sctp_assoc_choose_alter_transport(asoc,
1519 1519 asoc->init_last_sent_to);
1520 1520 asoc->init_last_sent_to = t;
1521 1521  
1522 1522  
... ... @@ -1665,17 +1665,16 @@
1665 1665 break;
1666 1666  
1667 1667 case SCTP_CMD_PART_DELIVER:
1668   - sctp_ulpq_partial_delivery(&asoc->ulpq, cmd->obj.ptr,
1669   - GFP_ATOMIC);
  1668 + sctp_ulpq_partial_delivery(&asoc->ulpq, GFP_ATOMIC);
1670 1669 break;
1671 1670  
1672 1671 case SCTP_CMD_RENEGE:
1673   - sctp_ulpq_renege(&asoc->ulpq, cmd->obj.ptr,
  1672 + sctp_ulpq_renege(&asoc->ulpq, cmd->obj.chunk,
1674 1673 GFP_ATOMIC);
1675 1674 break;
1676 1675  
1677 1676 case SCTP_CMD_SETUP_T4:
1678   - sctp_cmd_setup_t4(commands, asoc, cmd->obj.ptr);
  1677 + sctp_cmd_setup_t4(commands, asoc, cmd->obj.chunk);
1679 1678 break;
1680 1679  
1681 1680 case SCTP_CMD_PROCESS_OPERR:
... ... @@ -1734,8 +1733,8 @@
1734 1733 break;
1735 1734  
1736 1735 default:
1737   - pr_warn("Impossible command: %u, %p\n",
1738   - cmd->verb, cmd->obj.ptr);
  1736 + pr_warn("Impossible command: %u\n",
  1737 + cmd->verb);
1739 1738 break;
1740 1739 }
1741 1740  
... ... @@ -997,7 +997,6 @@
997 997  
998 998 /* Partial deliver the first message as there is pressure on rwnd. */
999 999 void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
1000   - struct sctp_chunk *chunk,
1001 1000 gfp_t gfp)
1002 1001 {
1003 1002 struct sctp_ulpevent *event;
... ... @@ -1060,7 +1059,7 @@
1060 1059 sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
1061 1060 sctp_ulpq_tail_data(ulpq, chunk, gfp);
1062 1061  
1063   - sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
  1062 + sctp_ulpq_partial_delivery(ulpq, gfp);
1064 1063 }
1065 1064  
1066 1065 sk_mem_reclaim(asoc->base.sk);