Commit 20c24bf24ddf91fd9ab5d827fdd0fe14397de412
Committed by
Anthony Liguori
1 parent
ef2d54d8
slirp: tftp: Refactor tftp_handle_rrq
Specifically make the filename extraction more readable, and always report errors back to the client. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Showing
1 changed file
with
21 additions
and
28 deletions
slirp/tftp.c
| @@ -27,7 +27,7 @@ | @@ -27,7 +27,7 @@ | ||
| 27 | 27 | ||
| 28 | struct tftp_session { | 28 | struct tftp_session { |
| 29 | int in_use; | 29 | int in_use; |
| 30 | - unsigned char filename[TFTP_FILENAME_MAX]; | 30 | + char filename[TFTP_FILENAME_MAX]; |
| 31 | 31 | ||
| 32 | struct in_addr client_ip; | 32 | struct in_addr client_ip; |
| 33 | u_int16_t client_port; | 33 | u_int16_t client_port; |
| @@ -273,8 +273,8 @@ static int tftp_send_data(struct tftp_session *spt, | @@ -273,8 +273,8 @@ static int tftp_send_data(struct tftp_session *spt, | ||
| 273 | static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | 273 | static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) |
| 274 | { | 274 | { |
| 275 | struct tftp_session *spt; | 275 | struct tftp_session *spt; |
| 276 | - int s, k, n; | ||
| 277 | - u_int8_t *src, *dst; | 276 | + int s, k; |
| 277 | + char *req_fname; | ||
| 278 | 278 | ||
| 279 | s = tftp_session_allocate(tp); | 279 | s = tftp_session_allocate(tp); |
| 280 | 280 | ||
| @@ -290,37 +290,31 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | @@ -290,37 +290,31 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | ||
| 290 | return; | 290 | return; |
| 291 | } | 291 | } |
| 292 | 292 | ||
| 293 | - src = tp->x.tp_buf; | ||
| 294 | - dst = spt->filename; | ||
| 295 | - n = pktlen - ((uint8_t *)&tp->x.tp_buf[0] - (uint8_t *)tp); | 293 | + /* skip header fields */ |
| 294 | + k = 0; | ||
| 295 | + pktlen -= ((uint8_t *)&tp->x.tp_buf[0] - (uint8_t *)tp); | ||
| 296 | 296 | ||
| 297 | /* get name */ | 297 | /* get name */ |
| 298 | + req_fname = spt->filename; | ||
| 298 | 299 | ||
| 299 | - for (k = 0; k < n; k++) { | ||
| 300 | - if (k < TFTP_FILENAME_MAX) { | ||
| 301 | - dst[k] = src[k]; | ||
| 302 | - } | ||
| 303 | - else { | 300 | + while (1) { |
| 301 | + if (k >= TFTP_FILENAME_MAX || k >= pktlen) { | ||
| 302 | + tftp_send_error(spt, 2, "Access violation", tp); | ||
| 304 | return; | 303 | return; |
| 305 | } | 304 | } |
| 306 | - | ||
| 307 | - if (src[k] == '\0') { | 305 | + req_fname[k] = (char)tp->x.tp_buf[k]; |
| 306 | + if (req_fname[k++] == '\0') { | ||
| 308 | break; | 307 | break; |
| 309 | } | 308 | } |
| 310 | } | 309 | } |
| 311 | 310 | ||
| 312 | - if (k >= n) { | ||
| 313 | - return; | ||
| 314 | - } | ||
| 315 | - | ||
| 316 | - k++; | ||
| 317 | - | ||
| 318 | /* check mode */ | 311 | /* check mode */ |
| 319 | - if ((n - k) < 6) { | 312 | + if ((pktlen - k) < 6) { |
| 313 | + tftp_send_error(spt, 2, "Access violation", tp); | ||
| 320 | return; | 314 | return; |
| 321 | } | 315 | } |
| 322 | 316 | ||
| 323 | - if (memcmp(&src[k], "octet\0", 6) != 0) { | 317 | + if (memcmp(&tp->x.tp_buf[k], "octet\0", 6) != 0) { |
| 324 | tftp_send_error(spt, 4, "Unsupported transfer mode", tp); | 318 | tftp_send_error(spt, 4, "Unsupported transfer mode", tp); |
| 325 | return; | 319 | return; |
| 326 | } | 320 | } |
| @@ -337,29 +331,28 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | @@ -337,29 +331,28 @@ static void tftp_handle_rrq(struct tftp_t *tp, int pktlen) | ||
| 337 | } | 331 | } |
| 338 | 332 | ||
| 339 | /* check if the file exists */ | 333 | /* check if the file exists */ |
| 340 | - | ||
| 341 | - if (tftp_read_data(spt, 0, spt->filename, 0) < 0) { | 334 | + if (tftp_read_data(spt, 0, NULL, 0) < 0) { |
| 342 | tftp_send_error(spt, 1, "File not found", tp); | 335 | tftp_send_error(spt, 1, "File not found", tp); |
| 343 | return; | 336 | return; |
| 344 | } | 337 | } |
| 345 | 338 | ||
| 346 | - if (src[n - 1] != 0) { | 339 | + if (tp->x.tp_buf[pktlen - 1] != 0) { |
| 347 | tftp_send_error(spt, 2, "Access violation", tp); | 340 | tftp_send_error(spt, 2, "Access violation", tp); |
| 348 | return; | 341 | return; |
| 349 | } | 342 | } |
| 350 | 343 | ||
| 351 | - while (k < n) { | 344 | + while (k < pktlen) { |
| 352 | const char *key, *value; | 345 | const char *key, *value; |
| 353 | 346 | ||
| 354 | - key = (char *)src + k; | 347 | + key = (const char *)&tp->x.tp_buf[k]; |
| 355 | k += strlen(key) + 1; | 348 | k += strlen(key) + 1; |
| 356 | 349 | ||
| 357 | - if (k >= n) { | 350 | + if (k >= pktlen) { |
| 358 | tftp_send_error(spt, 2, "Access violation", tp); | 351 | tftp_send_error(spt, 2, "Access violation", tp); |
| 359 | return; | 352 | return; |
| 360 | } | 353 | } |
| 361 | 354 | ||
| 362 | - value = (char *)src + k; | 355 | + value = (const char *)&tp->x.tp_buf[k]; |
| 363 | k += strlen(value) + 1; | 356 | k += strlen(value) + 1; |
| 364 | 357 | ||
| 365 | if (strcmp(key, "tsize") == 0) { | 358 | if (strcmp(key, "tsize") == 0) { |