Browse Source

Fix the android-cloexec-* warnings in bootable/recovery

Add the O_CLOEXEC or 'e' accordingly.

Bug: 63510015
Test: recovery tests pass
Change-Id: I7094bcc6af22c9687eb535116b2ca6a59178b303
Tianjie Xu 4 years ago
parent
commit
de6735e80c
7 changed files with 115 additions and 116 deletions
  1. 1
    1
      install.cpp
  2. 1
    1
      minui/resources.cpp
  3. 15
    15
      recovery-persist.cpp
  4. 24
    24
      recovery.cpp
  5. 2
    2
      tests/component/updater_test.cpp
  6. 1
    1
      tests/manual/recovery_test.cpp
  7. 71
    72
      verifier.cpp

+ 1
- 1
install.cpp View File

@@ -265,7 +265,7 @@ int update_binary_command(const std::string& package, ZipArchiveHandle zip,
265 265
   }
266 266
 
267 267
   unlink(binary_path.c_str());
268
-  int fd = creat(binary_path.c_str(), 0755);
268
+  int fd = open(binary_path.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 0755);
269 269
   if (fd == -1) {
270 270
     PLOG(ERROR) << "Failed to create " << binary_path;
271 271
     return INSTALL_ERROR;

+ 1
- 1
minui/resources.cpp View File

@@ -56,7 +56,7 @@ static int open_png(const char* name, png_structp* png_ptr, png_infop* info_ptr,
56 56
 
57 57
     snprintf(resPath, sizeof(resPath)-1, "/res/images/%s.png", name);
58 58
     resPath[sizeof(resPath)-1] = '\0';
59
-    FILE* fp = fopen(resPath, "rb");
59
+    FILE* fp = fopen(resPath, "rbe");
60 60
     if (fp == NULL) {
61 61
         result = -1;
62 62
         goto exit;

+ 15
- 15
recovery-persist.cpp View File

@@ -59,21 +59,21 @@ static void check_and_fclose(FILE *fp, const char *name) {
59 59
 }
60 60
 
61 61
 static void copy_file(const char* source, const char* destination) {
62
-    FILE* dest_fp = fopen(destination, "w");
63
-    if (dest_fp == nullptr) {
64
-        PLOG(ERROR) << "Can't open " << destination;
65
-    } else {
66
-        FILE* source_fp = fopen(source, "r");
67
-        if (source_fp != nullptr) {
68
-            char buf[4096];
69
-            size_t bytes;
70
-            while ((bytes = fread(buf, 1, sizeof(buf), source_fp)) != 0) {
71
-                fwrite(buf, 1, bytes, dest_fp);
72
-            }
73
-            check_and_fclose(source_fp, source);
74
-        }
75
-        check_and_fclose(dest_fp, destination);
62
+  FILE* dest_fp = fopen(destination, "we");
63
+  if (dest_fp == nullptr) {
64
+    PLOG(ERROR) << "Can't open " << destination;
65
+  } else {
66
+    FILE* source_fp = fopen(source, "re");
67
+    if (source_fp != nullptr) {
68
+      char buf[4096];
69
+      size_t bytes;
70
+      while ((bytes = fread(buf, 1, sizeof(buf), source_fp)) != 0) {
71
+        fwrite(buf, 1, bytes, dest_fp);
72
+      }
73
+      check_and_fclose(source_fp, source);
76 74
     }
75
+    check_and_fclose(dest_fp, destination);
76
+  }
77 77
 }
78 78
 
79 79
 static bool rotated = false;
@@ -120,7 +120,7 @@ int main(int argc, char **argv) {
120 120
      */
121 121
     bool has_cache = false;
122 122
     static const char mounts_file[] = "/proc/mounts";
123
-    FILE *fp = fopen(mounts_file, "r");
123
+    FILE* fp = fopen(mounts_file, "re");
124 124
     if (!fp) {
125 125
         PLOG(ERROR) << "failed to open " << mounts_file;
126 126
     } else {

+ 24
- 24
recovery.cpp View File

@@ -249,7 +249,7 @@ static void redirect_stdio(const char* filename) {
249 249
         auto start = std::chrono::steady_clock::now();
250 250
 
251 251
         // Child logger to actually write to the log file.
252
-        FILE* log_fp = fopen(filename, "a");
252
+        FILE* log_fp = fopen(filename, "ae");
253 253
         if (log_fp == nullptr) {
254 254
             PLOG(ERROR) << "fopen \"" << filename << "\" failed";
255 255
             close(pipefd[0]);
@@ -418,27 +418,27 @@ static void copy_log_file_to_pmsg(const char* source, const char* destination) {
418 418
 static off_t tmplog_offset = 0;
419 419
 
420 420
 static void copy_log_file(const char* source, const char* destination, bool append) {
421
-    FILE* dest_fp = fopen_path(destination, append ? "a" : "w");
422
-    if (dest_fp == nullptr) {
423
-        PLOG(ERROR) << "Can't open " << destination;
424
-    } else {
425
-        FILE* source_fp = fopen(source, "r");
426
-        if (source_fp != nullptr) {
427
-            if (append) {
428
-                fseeko(source_fp, tmplog_offset, SEEK_SET);  // Since last write
429
-            }
430
-            char buf[4096];
431
-            size_t bytes;
432
-            while ((bytes = fread(buf, 1, sizeof(buf), source_fp)) != 0) {
433
-                fwrite(buf, 1, bytes, dest_fp);
434
-            }
435
-            if (append) {
436
-                tmplog_offset = ftello(source_fp);
437
-            }
438
-            check_and_fclose(source_fp, source);
439
-        }
440
-        check_and_fclose(dest_fp, destination);
421
+  FILE* dest_fp = fopen_path(destination, append ? "ae" : "we");
422
+  if (dest_fp == nullptr) {
423
+    PLOG(ERROR) << "Can't open " << destination;
424
+  } else {
425
+    FILE* source_fp = fopen(source, "re");
426
+    if (source_fp != nullptr) {
427
+      if (append) {
428
+        fseeko(source_fp, tmplog_offset, SEEK_SET);  // Since last write
429
+      }
430
+      char buf[4096];
431
+      size_t bytes;
432
+      while ((bytes = fread(buf, 1, sizeof(buf), source_fp)) != 0) {
433
+        fwrite(buf, 1, bytes, dest_fp);
434
+      }
435
+      if (append) {
436
+        tmplog_offset = ftello(source_fp);
437
+      }
438
+      check_and_fclose(source_fp, source);
441 439
     }
440
+    check_and_fclose(dest_fp, destination);
441
+  }
442 442
 }
443 443
 
444 444
 static void copy_logs() {
@@ -487,7 +487,7 @@ static void finish_recovery() {
487 487
     if (!locale.empty() && has_cache) {
488 488
         LOG(INFO) << "Saving locale \"" << locale << "\"";
489 489
 
490
-        FILE* fp = fopen_path(LOCALE_FILE, "w");
490
+        FILE* fp = fopen_path(LOCALE_FILE, "we");
491 491
         if (!android::base::WriteStringToFd(locale, fileno(fp))) {
492 492
             PLOG(ERROR) << "Failed to save locale to " << LOCALE_FILE;
493 493
         }
@@ -551,7 +551,7 @@ static bool erase_volume(const char* volume) {
551 551
             }
552 552
 
553 553
             std::string data(sb.st_size, '\0');
554
-            FILE* f = fopen(path.c_str(), "rb");
554
+            FILE* f = fopen(path.c_str(), "rbe");
555 555
             fread(&data[0], 1, data.size(), f);
556 556
             fclose(f);
557 557
 
@@ -579,7 +579,7 @@ static bool erase_volume(const char* volume) {
579 579
       ui->Print("Failed to make convert_fbe dir %s\n", strerror(errno));
580 580
       return true;
581 581
     }
582
-    FILE* f = fopen(CONVERT_FBE_FILE, "wb");
582
+    FILE* f = fopen(CONVERT_FBE_FILE, "wbe");
583 583
     if (!f) {
584 584
       ui->Print("Failed to convert to file encryption %s\n", strerror(errno));
585 585
       return true;

+ 2
- 2
tests/component/updater_test.cpp View File

@@ -485,7 +485,7 @@ TEST_F(UpdaterTest, block_image_update) {
485 485
   UpdaterInfo updater_info;
486 486
   updater_info.package_zip = handle;
487 487
   TemporaryFile temp_pipe;
488
-  updater_info.cmd_pipe = fopen(temp_pipe.path, "wb");
488
+  updater_info.cmd_pipe = fopen(temp_pipe.path, "wbe");
489 489
   updater_info.package_zip_addr = map.addr;
490 490
   updater_info.package_zip_len = map.length;
491 491
 
@@ -561,7 +561,7 @@ TEST_F(UpdaterTest, new_data_short_write) {
561 561
   UpdaterInfo updater_info;
562 562
   updater_info.package_zip = handle;
563 563
   TemporaryFile temp_pipe;
564
-  updater_info.cmd_pipe = fopen(temp_pipe.path, "wb");
564
+  updater_info.cmd_pipe = fopen(temp_pipe.path, "wbe");
565 565
   updater_info.package_zip_addr = map.addr;
566 566
   updater_info.package_zip_len = map.length;
567 567
 

+ 1
- 1
tests/manual/recovery_test.cpp View File

@@ -141,7 +141,7 @@ class ResourceTest : public testing::TestWithParam<std::string> {
141 141
   // under recovery.
142 142
   void SetUp() override {
143 143
     std::string file_path = GetParam();
144
-    fp = fopen(file_path.c_str(), "rb");
144
+    fp = fopen(file_path.c_str(), "rbe");
145 145
     ASSERT_NE(nullptr, fp);
146 146
 
147 147
     unsigned char header[8];

+ 71
- 72
verifier.cpp View File

@@ -474,81 +474,80 @@ std::unique_ptr<EC_KEY, ECKEYDeleter> parse_ec_key(FILE* file) {
474 474
 // Otherwise returns false if the file failed to parse, or if it contains zero
475 475
 // keys. The contents in certs would be unspecified on failure.
476 476
 bool load_keys(const char* filename, std::vector<Certificate>& certs) {
477
-    std::unique_ptr<FILE, decltype(&fclose)> f(fopen(filename, "r"), fclose);
478
-    if (!f) {
479
-        PLOG(ERROR) << "error opening " << filename;
480
-        return false;
481
-    }
482
-
483
-    while (true) {
484
-        certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr);
485
-        Certificate& cert = certs.back();
486
-        uint32_t exponent = 0;
487
-
488
-        char start_char;
489
-        if (fscanf(f.get(), " %c", &start_char) != 1) return false;
490
-        if (start_char == '{') {
491
-            // a version 1 key has no version specifier.
492
-            cert.key_type = Certificate::KEY_TYPE_RSA;
493
-            exponent = 3;
494
-            cert.hash_len = SHA_DIGEST_LENGTH;
495
-        } else if (start_char == 'v') {
496
-            int version;
497
-            if (fscanf(f.get(), "%d {", &version) != 1) return false;
498
-            switch (version) {
499
-                case 2:
500
-                    cert.key_type = Certificate::KEY_TYPE_RSA;
501
-                    exponent = 65537;
502
-                    cert.hash_len = SHA_DIGEST_LENGTH;
503
-                    break;
504
-                case 3:
505
-                    cert.key_type = Certificate::KEY_TYPE_RSA;
506
-                    exponent = 3;
507
-                    cert.hash_len = SHA256_DIGEST_LENGTH;
508
-                    break;
509
-                case 4:
510
-                    cert.key_type = Certificate::KEY_TYPE_RSA;
511
-                    exponent = 65537;
512
-                    cert.hash_len = SHA256_DIGEST_LENGTH;
513
-                    break;
514
-                case 5:
515
-                    cert.key_type = Certificate::KEY_TYPE_EC;
516
-                    cert.hash_len = SHA256_DIGEST_LENGTH;
517
-                    break;
518
-                default:
519
-                    return false;
520
-            }
521
-        }
477
+  std::unique_ptr<FILE, decltype(&fclose)> f(fopen(filename, "re"), fclose);
478
+  if (!f) {
479
+    PLOG(ERROR) << "error opening " << filename;
480
+    return false;
481
+  }
522 482
 
523
-        if (cert.key_type == Certificate::KEY_TYPE_RSA) {
524
-            cert.rsa = parse_rsa_key(f.get(), exponent);
525
-            if (!cert.rsa) {
526
-              return false;
527
-            }
483
+  while (true) {
484
+    certs.emplace_back(0, Certificate::KEY_TYPE_RSA, nullptr, nullptr);
485
+    Certificate& cert = certs.back();
486
+    uint32_t exponent = 0;
487
+
488
+    char start_char;
489
+    if (fscanf(f.get(), " %c", &start_char) != 1) return false;
490
+    if (start_char == '{') {
491
+      // a version 1 key has no version specifier.
492
+      cert.key_type = Certificate::KEY_TYPE_RSA;
493
+      exponent = 3;
494
+      cert.hash_len = SHA_DIGEST_LENGTH;
495
+    } else if (start_char == 'v') {
496
+      int version;
497
+      if (fscanf(f.get(), "%d {", &version) != 1) return false;
498
+      switch (version) {
499
+        case 2:
500
+          cert.key_type = Certificate::KEY_TYPE_RSA;
501
+          exponent = 65537;
502
+          cert.hash_len = SHA_DIGEST_LENGTH;
503
+          break;
504
+        case 3:
505
+          cert.key_type = Certificate::KEY_TYPE_RSA;
506
+          exponent = 3;
507
+          cert.hash_len = SHA256_DIGEST_LENGTH;
508
+          break;
509
+        case 4:
510
+          cert.key_type = Certificate::KEY_TYPE_RSA;
511
+          exponent = 65537;
512
+          cert.hash_len = SHA256_DIGEST_LENGTH;
513
+          break;
514
+        case 5:
515
+          cert.key_type = Certificate::KEY_TYPE_EC;
516
+          cert.hash_len = SHA256_DIGEST_LENGTH;
517
+          break;
518
+        default:
519
+          return false;
520
+      }
521
+    }
528 522
 
529
-            LOG(INFO) << "read key e=" << exponent << " hash=" << cert.hash_len;
530
-        } else if (cert.key_type == Certificate::KEY_TYPE_EC) {
531
-            cert.ec = parse_ec_key(f.get());
532
-            if (!cert.ec) {
533
-              return false;
534
-            }
535
-        } else {
536
-            LOG(ERROR) << "Unknown key type " << cert.key_type;
537
-            return false;
538
-        }
523
+    if (cert.key_type == Certificate::KEY_TYPE_RSA) {
524
+      cert.rsa = parse_rsa_key(f.get(), exponent);
525
+      if (!cert.rsa) {
526
+        return false;
527
+      }
539 528
 
540
-        // if the line ends in a comma, this file has more keys.
541
-        int ch = fgetc(f.get());
542
-        if (ch == ',') {
543
-            // more keys to come.
544
-            continue;
545
-        } else if (ch == EOF) {
546
-            break;
547
-        } else {
548
-            LOG(ERROR) << "unexpected character between keys";
549
-            return false;
550
-        }
529
+      LOG(INFO) << "read key e=" << exponent << " hash=" << cert.hash_len;
530
+    } else if (cert.key_type == Certificate::KEY_TYPE_EC) {
531
+      cert.ec = parse_ec_key(f.get());
532
+      if (!cert.ec) {
533
+        return false;
534
+      }
535
+    } else {
536
+      LOG(ERROR) << "Unknown key type " << cert.key_type;
537
+      return false;
551 538
     }
552 539
 
553
-    return true;
540
+    // if the line ends in a comma, this file has more keys.
541
+    int ch = fgetc(f.get());
542
+    if (ch == ',') {
543
+      // more keys to come.
544
+      continue;
545
+    } else if (ch == EOF) {
546
+      break;
547
+    } else {
548
+      LOG(ERROR) << "unexpected character between keys";
549
+      return false;
550
+    }
551
+  }
552
+  return true;
554 553
 }