Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(15)

Side by Side Diff: components/breakpad/browser/crash_handler_host_linux.cc

Issue 283643004: Remove SCM_CREDENTIALS fallback code from breakpad (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « components/breakpad/app/breakpad_linux.cc ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/breakpad/browser/crash_handler_host_linux.h" 5 #include "components/breakpad/browser/crash_handler_host_linux.h"
6 6
7 #include <stdint.h> 7 #include <stdint.h>
8 #include <stdlib.h> 8 #include <stdlib.h>
9 #include <sys/socket.h> 9 #include <sys/socket.h>
10 #include <sys/syscall.h> 10 #include <sys/syscall.h>
(...skipping 201 matching lines...) Expand 10 before | Expand all | Expand 10 after
212 LOG(ERROR) << "Received death signal message with the wrong size;" 212 LOG(ERROR) << "Received death signal message with the wrong size;"
213 << " msg.msg_controllen:" << msg.msg_controllen 213 << " msg.msg_controllen:" << msg.msg_controllen
214 << " msg.msg_flags:" << msg.msg_flags 214 << " msg.msg_flags:" << msg.msg_flags
215 << " kCrashContextSize:" << kCrashContextSize 215 << " kCrashContextSize:" << kCrashContextSize
216 << " kControlMsgSize:" << kControlMsgSize; 216 << " kControlMsgSize:" << kControlMsgSize;
217 return; 217 return;
218 } 218 }
219 219
220 // Walk the control payload an extract the file descriptor and validated pid. 220 // Walk the control payload an extract the file descriptor and validated pid.
221 pid_t crashing_pid = -1; 221 pid_t crashing_pid = -1;
222 int partner_fd = -1;
223 int signal_fd = -1; 222 int signal_fd = -1;
224 for (struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg); hdr; 223 for (struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg); hdr;
225 hdr = CMSG_NXTHDR(&msg, hdr)) { 224 hdr = CMSG_NXTHDR(&msg, hdr)) {
226 if (hdr->cmsg_level != SOL_SOCKET) 225 if (hdr->cmsg_level != SOL_SOCKET)
227 continue; 226 continue;
228 if (hdr->cmsg_type == SCM_RIGHTS) { 227 if (hdr->cmsg_type == SCM_RIGHTS) {
229 const unsigned len = hdr->cmsg_len - 228 const unsigned len = hdr->cmsg_len -
230 (((uint8_t*)CMSG_DATA(hdr)) - (uint8_t*)hdr); 229 (((uint8_t*)CMSG_DATA(hdr)) - (uint8_t*)hdr);
231 DCHECK_EQ(len % sizeof(int), 0u); 230 DCHECK_EQ(len % sizeof(int), 0u);
232 const unsigned num_fds = len / sizeof(int); 231 const unsigned num_fds = len / sizeof(int);
233 if (num_fds != 2) { 232 if (num_fds != 1) {
234 // A nasty process could try and send us too many descriptors and 233 // A nasty process could try and send us too many descriptors and
235 // force a leak. 234 // force a leak.
236 LOG(ERROR) << "Death signal contained wrong number of descriptors;" 235 LOG(ERROR) << "Death signal contained wrong number of descriptors;"
237 << " num_fds:" << num_fds; 236 << " num_fds:" << num_fds;
238 for (unsigned i = 0; i < num_fds; ++i) 237 for (unsigned i = 0; i < num_fds; ++i)
239 close(reinterpret_cast<int*>(CMSG_DATA(hdr))[i]); 238 close(reinterpret_cast<int*>(CMSG_DATA(hdr))[i]);
240 return; 239 return;
241 } else { 240 } else {
242 partner_fd = reinterpret_cast<int*>(CMSG_DATA(hdr))[0];
243 signal_fd = reinterpret_cast<int*>(CMSG_DATA(hdr))[1]; 241 signal_fd = reinterpret_cast<int*>(CMSG_DATA(hdr))[1];
jln (very slow on Chromium) 2014/05/13 00:11:58 Shouldn't this be [0]?
mdempsky 2014/05/13 00:14:20 Yep, already fixed in patch set 2. :)
244 } 242 }
245 } else if (hdr->cmsg_type == SCM_CREDENTIALS) { 243 } else if (hdr->cmsg_type == SCM_CREDENTIALS) {
246 const struct ucred *cred = 244 const struct ucred *cred =
247 reinterpret_cast<struct ucred*>(CMSG_DATA(hdr)); 245 reinterpret_cast<struct ucred*>(CMSG_DATA(hdr));
248 crashing_pid = cred->pid; 246 crashing_pid = cred->pid;
249 } 247 }
250 } 248 }
251 249
252 if (crashing_pid == -1 || partner_fd == -1 || signal_fd == -1) { 250 if (crashing_pid == -1 || signal_fd == -1) {
253 LOG(ERROR) << "Death signal message didn't contain all expected control" 251 LOG(ERROR) << "Death signal message didn't contain all expected control"
254 << " messages"; 252 << " messages";
255 if (partner_fd >= 0)
256 close(partner_fd);
257 if (signal_fd >= 0) 253 if (signal_fd >= 0)
258 close(signal_fd); 254 close(signal_fd);
259 return; 255 return;
260 } 256 }
261 257
262 // Kernel bug workaround (broken in 2.6.30 and 2.6.32, working in 2.6.38).
263 // The kernel doesn't translate PIDs in SCM_CREDENTIALS across PID
264 // namespaces. Thus |crashing_pid| might be garbage from our point of view.
265 // In the future we can remove this workaround, but we have to wait a couple
266 // of years to be sure that it's worked its way out into the world.
267 // TODO(thestig) Remove the workaround when Ubuntu Lucid is deprecated.
268
269 // The crashing process closes its copy of the signal_fd immediately after
270 // calling sendmsg(). We can thus not reliably look for with with
271 // FindProcessHoldingSocket(). But by necessity, it has to keep the
272 // partner_fd open until the crashdump is complete.
273 ino_t inode_number;
274 if (!base::FileDescriptorGetInode(&inode_number, partner_fd)) {
275 LOG(WARNING) << "Failed to get inode number for passed socket";
276 close(partner_fd);
277 close(signal_fd);
278 return;
279 }
280 close(partner_fd);
281
282 pid_t actual_crashing_pid = -1;
283 if (!base::FindProcessHoldingSocket(&actual_crashing_pid, inode_number)) {
284 LOG(WARNING) << "Failed to find process holding other end of crash reply "
285 "socket";
286 close(signal_fd);
287 return;
288 }
289
290 crashing_pid = actual_crashing_pid;
291
292 // The crashing TID set inside the compromised context via 258 // The crashing TID set inside the compromised context via
293 // sys_gettid() in ExceptionHandler::HandleSignal might be wrong (if 259 // sys_gettid() in ExceptionHandler::HandleSignal might be wrong (if
294 // the kernel supports PID namespacing) and may need to be 260 // the kernel supports PID namespacing) and may need to be
295 // translated. 261 // translated.
296 // 262 //
297 // We expect the crashing thread to be in sys_read(), waiting for us to 263 // We expect the crashing thread to be in sys_read(), waiting for us to
298 // write to |signal_fd|. Most newer kernels where we have the different pid 264 // write to |signal_fd|. Most newer kernels where we have the different pid
299 // namespaces also have /proc/[pid]/syscall, so we can look through 265 // namespaces also have /proc/[pid]/syscall, so we can look through
300 // |actual_crashing_pid|'s thread group and find the thread that's in the 266 // |actual_crashing_pid|'s thread group and find the thread that's in the
301 // read syscall with the right arguments. 267 // read syscall with the right arguments.
(...skipping 154 matching lines...) Expand 10 before | Expand all | Expand 10 after
456 // no-ops. 422 // no-ops.
457 shutting_down_ = true; 423 shutting_down_ = true;
458 uploader_thread_->Stop(); 424 uploader_thread_->Stop();
459 } 425 }
460 426
461 bool CrashHandlerHostLinux::IsShuttingDown() const { 427 bool CrashHandlerHostLinux::IsShuttingDown() const {
462 return shutting_down_; 428 return shutting_down_;
463 } 429 }
464 430
465 } // namespace breakpad 431 } // namespace breakpad
OLDNEW
« no previous file with comments | « components/breakpad/app/breakpad_linux.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698